|
Takashi Iwai |
62c76a |
From 24990423267ec283b9d86f07f362b753eb9b0ed5 Mon Sep 17 00:00:00 2001
|
|
Takashi Iwai |
62c76a |
From: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
|
|
Takashi Iwai |
62c76a |
Date: Wed, 26 May 2021 08:39:37 -0400
|
|
Takashi Iwai |
62c76a |
Subject: [PATCH] i2c: s3c2410: fix possible NULL pointer deref on read message after write
|
|
Takashi Iwai |
62c76a |
Git-commit: 24990423267ec283b9d86f07f362b753eb9b0ed5
|
|
Takashi Iwai |
62c76a |
Patch-mainline: v5.13-rc4
|
|
Takashi Iwai |
62c76a |
References: git-fixes
|
|
Takashi Iwai |
62c76a |
|
|
Takashi Iwai |
62c76a |
Interrupt handler processes multiple message write requests one after
|
|
Takashi Iwai |
62c76a |
another, till the driver message queue is drained. However if driver
|
|
Takashi Iwai |
62c76a |
encounters a read message without preceding START, it stops the I2C
|
|
Takashi Iwai |
62c76a |
transfer as it is an invalid condition for the controller. At least the
|
|
Takashi Iwai |
62c76a |
comment describes a requirement "the controller forces us to send a new
|
|
Takashi Iwai |
62c76a |
START when we change direction". This stop results in clearing the
|
|
Takashi Iwai |
62c76a |
message queue (i2c->msg = NULL).
|
|
Takashi Iwai |
62c76a |
|
|
Takashi Iwai |
62c76a |
The code however immediately jumped back to label "retry_write" which
|
|
Takashi Iwai |
62c76a |
dereferenced the "i2c->msg" making it a possible NULL pointer
|
|
Takashi Iwai |
62c76a |
dereference.
|
|
Takashi Iwai |
62c76a |
|
|
Takashi Iwai |
62c76a |
The Coverity analysis:
|
|
Takashi Iwai |
62c76a |
1. Condition !is_msgend(i2c), taking false branch.
|
|
Takashi Iwai |
62c76a |
if (!is_msgend(i2c)) {
|
|
Takashi Iwai |
62c76a |
|
|
Takashi Iwai |
62c76a |
2. Condition !is_lastmsg(i2c), taking true branch.
|
|
Takashi Iwai |
62c76a |
} else if (!is_lastmsg(i2c)) {
|
|
Takashi Iwai |
62c76a |
|
|
Takashi Iwai |
62c76a |
3. Condition i2c->msg->flags & 1, taking true branch.
|
|
Takashi Iwai |
62c76a |
if (i2c->msg->flags & I2C_M_RD) {
|
|
Takashi Iwai |
62c76a |
|
|
Takashi Iwai |
62c76a |
4. write_zero_model: Passing i2c to s3c24xx_i2c_stop, which sets i2c->msg to NULL.
|
|
Takashi Iwai |
62c76a |
s3c24xx_i2c_stop(i2c, -EINVAL);
|
|
Takashi Iwai |
62c76a |
|
|
Takashi Iwai |
62c76a |
5. Jumping to label retry_write.
|
|
Takashi Iwai |
62c76a |
goto retry_write;
|
|
Takashi Iwai |
62c76a |
|
|
Takashi Iwai |
62c76a |
6. var_deref_model: Passing i2c to is_msgend, which dereferences null i2c->msg.
|
|
Takashi Iwai |
62c76a |
if (!is_msgend(i2c)) {"
|
|
Takashi Iwai |
62c76a |
|
|
Takashi Iwai |
62c76a |
All previous calls to s3c24xx_i2c_stop() in this interrupt service
|
|
Takashi Iwai |
62c76a |
routine are followed by jumping to end of function (acknowledging
|
|
Takashi Iwai |
62c76a |
the interrupt and returning). This seems a reasonable choice also here
|
|
Takashi Iwai |
62c76a |
since message buffer was entirely emptied.
|
|
Takashi Iwai |
62c76a |
|
|
Takashi Iwai |
62c76a |
Addresses-coverity: Explicit null dereferenced
|
|
Takashi Iwai |
62c76a |
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
|
|
Takashi Iwai |
62c76a |
Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>
|
|
Takashi Iwai |
62c76a |
Signed-off-by: Wolfram Sang <wsa@kernel.org>
|
|
Takashi Iwai |
62c76a |
Acked-by: Takashi Iwai <tiwai@suse.de>
|
|
Takashi Iwai |
62c76a |
|
|
Takashi Iwai |
62c76a |
---
|
|
Takashi Iwai |
62c76a |
drivers/i2c/busses/i2c-s3c2410.c | 3 +++
|
|
Takashi Iwai |
62c76a |
1 file changed, 3 insertions(+)
|
|
Takashi Iwai |
62c76a |
|
|
Takashi Iwai |
62c76a |
diff --git a/drivers/i2c/busses/i2c-s3c2410.c b/drivers/i2c/busses/i2c-s3c2410.c
|
|
Takashi Iwai |
62c76a |
index ab928613afba..4d82761e1585 100644
|
|
Takashi Iwai |
62c76a |
--- a/drivers/i2c/busses/i2c-s3c2410.c
|
|
Takashi Iwai |
62c76a |
+++ b/drivers/i2c/busses/i2c-s3c2410.c
|
|
Takashi Iwai |
62c76a |
@@ -480,7 +480,10 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat)
|
|
Takashi Iwai |
62c76a |
* forces us to send a new START
|
|
Takashi Iwai |
62c76a |
* when we change direction
|
|
Takashi Iwai |
62c76a |
*/
|
|
Takashi Iwai |
62c76a |
+ dev_dbg(i2c->dev,
|
|
Takashi Iwai |
62c76a |
+ "missing START before write->read\n");
|
|
Takashi Iwai |
62c76a |
s3c24xx_i2c_stop(i2c, -EINVAL);
|
|
Takashi Iwai |
62c76a |
+ break;
|
|
Takashi Iwai |
62c76a |
}
|
|
Takashi Iwai |
62c76a |
|
|
Takashi Iwai |
62c76a |
goto retry_write;
|
|
Takashi Iwai |
62c76a |
--
|
|
Takashi Iwai |
62c76a |
2.26.2
|
|
Takashi Iwai |
62c76a |
|