Skip to content

Commit 2499042

Browse files
krzkwsakernel
authored andcommitted
i2c: s3c2410: fix possible NULL pointer deref on read message after write
Interrupt handler processes multiple message write requests one after another, till the driver message queue is drained. However if driver encounters a read message without preceding START, it stops the I2C transfer as it is an invalid condition for the controller. At least the comment describes a requirement "the controller forces us to send a new START when we change direction". This stop results in clearing the message queue (i2c->msg = NULL). The code however immediately jumped back to label "retry_write" which dereferenced the "i2c->msg" making it a possible NULL pointer dereference. The Coverity analysis: 1. Condition !is_msgend(i2c), taking false branch. if (!is_msgend(i2c)) { 2. Condition !is_lastmsg(i2c), taking true branch. } else if (!is_lastmsg(i2c)) { 3. Condition i2c->msg->flags & 1, taking true branch. if (i2c->msg->flags & I2C_M_RD) { 4. write_zero_model: Passing i2c to s3c24xx_i2c_stop, which sets i2c->msg to NULL. s3c24xx_i2c_stop(i2c, -EINVAL); 5. Jumping to label retry_write. goto retry_write; 6. var_deref_model: Passing i2c to is_msgend, which dereferences null i2c->msg. if (!is_msgend(i2c)) {" All previous calls to s3c24xx_i2c_stop() in this interrupt service routine are followed by jumping to end of function (acknowledging the interrupt and returning). This seems a reasonable choice also here since message buffer was entirely emptied. Addresses-Coverity: Explicit null dereferenced Fixes: 1da177e ("Linux-2.6.12-rc2") Signed-off-by: Krzysztof Kozlowski <[email protected]> Signed-off-by: Wolfram Sang <[email protected]>
1 parent fed1bd5 commit 2499042

File tree

1 file changed

+3
-0
lines changed

1 file changed

+3
-0
lines changed

drivers/i2c/busses/i2c-s3c2410.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -480,7 +480,10 @@ static int i2c_s3c_irq_nextbyte(struct s3c24xx_i2c *i2c, unsigned long iicstat)
480480
* forces us to send a new START
481481
* when we change direction
482482
*/
483+
dev_dbg(i2c->dev,
484+
"missing START before write->read\n");
483485
s3c24xx_i2c_stop(i2c, -EINVAL);
486+
break;
484487
}
485488

486489
goto retry_write;

0 commit comments

Comments
 (0)