Re: [PATCH] i2c-stub: Avoid an array overrun on I2C block transfers

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 07/13/2014 09:24 AM, Jean Delvare wrote:
Hi Guenter,

On Sun, 13 Jul 2014 08:44:24 -0700, Guenter Roeck wrote:
On 07/13/2014 08:17 AM, Jean Delvare wrote:
I2C block transfers can have a size up to 32 bytes. If starting close
to the end of the address space, there may not be enough room to write
that many bytes (on I2C block writes) or not enough bytes to be read
(on I2C block reads.) In that case, we must shorten the transfer so
that it does not exceed the address space.

Signed-off-by: Jean Delvare <jdelvare@xxxxxxx>
Cc: Guenter Roeck <linux@xxxxxxxxxxxx>
Cc: Wolfram Sang <wsa@xxxxxxxxxxxxx>
---
   drivers/i2c/i2c-stub.c |    2 ++
   1 file changed, 2 insertions(+)

--- linux-3.16-rc4.orig/drivers/i2c/i2c-stub.c	2014-07-12 11:56:30.933096483 +0200
+++ linux-3.16-rc4/drivers/i2c/i2c-stub.c	2014-07-13 17:01:02.891235856 +0200
@@ -220,6 +220,8 @@ static s32 stub_xfer(struct i2c_adapter
   		 * We ignore banks here, because banked chips don't use I2C
   		 * block transfers
   		 */
+		if (data->block[0] > 256 - command)	/* Avoid overrun */
+			data->block[0] = 256 - command;

is it safe to overwrite data->block[0], or should it be something
like the following ?

		if (data->block[0] > 256 - command)	/* Avoid overrun */
			len = 256 - command;
		else
			len = data->block[0];

It's not only safe, it is desired. Otherwise the caller doesn't know
this is a short read, and it may take the end of the block buffer for
actual data. Check the code in
i2c-core.c:i2c_smbus_read_i2c_block_data(), you'll see it uses and even
returns block[0]. Same for writes, that's the only way to notify the
caller of short writes.

Also, wonder what happens in the real world if anyone does that.
Would the write stop at offset 255, or would it wrap and write from 0 ?

Depends on the chip, I've seen both implementations.

It doesn't really matter what i2c-stub does, as device drivers should
never do that. I just did not want to risk data leak or corruption in
case it ever happens.


Ok, makes sense.

Reviewed-by: Guenter Roeck <linux@xxxxxxxxxxxx>

Thanks,
Guenter

--
To unsubscribe from this list: send the line "unsubscribe linux-i2c" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux GPIO]     [Linux SPI]     [Linux Hardward Monitoring]     [LM Sensors]     [Linux USB Devel]     [Linux Media]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux