Re: [PATCH 1/2] i2c: designware: Only check the first byte for SMBus block read length
From: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Date: 2020-06-15 09:40:28
Also in:
linux-i2c, lkml
On Sun, Jun 14, 2020 at 02:02:54PM -0700, Sultan Alsawaf wrote:
From: Sultan Alsawaf <redacted> SMBus block reads can be broken because the read function will just skip over bytes it doesn't like until reaching a byte that conforms to the length restrictions for block reads. This is problematic when it isn't known if the incoming payload is indeed a conforming block read. According to the SMBus specification, block reads will only send the payload length in the first byte, so we can fix this by only considering the first byte in a sequence for block read length purposes.
I'm wondering if this overlaps with [1]. AFAIU that one is also makes sure that the length is not a garbage. [1]: https://lore.kernel.org/linux-i2c/20200613104109.2989-1-mans@mansr.com/T/#u (local)
quoted hunk ↗ jump to hunk
Fixes: c3ae106050b9 ("i2c: designware: Implement support for SMBus block read and write") Signed-off-by: Sultan Alsawaf <redacted> --- drivers/i2c/busses/i2c-designware-master.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)diff --git a/drivers/i2c/busses/i2c-designware-master.c b/drivers/i2c/busses/i2c-designware-master.c index d6425ad6e6a3..16d38b8fc19a 100644 --- a/drivers/i2c/busses/i2c-designware-master.c +++ b/drivers/i2c/busses/i2c-designware-master.c@@ -398,7 +398,6 @@ i2c_dw_recv_len(struct dw_i2c_dev *dev, u8 len) len += (flags & I2C_CLIENT_PEC) ? 2 : 1; dev->tx_buf_len = len - min_t(u8, len, dev->rx_outstanding); msgs[dev->msg_read_idx].len = len; - msgs[dev->msg_read_idx].flags &= ~I2C_M_RECV_LEN; return len; }@@ -430,10 +429,11 @@ i2c_dw_read(struct dw_i2c_dev *dev) u32 flags = msgs[dev->msg_read_idx].flags; regmap_read(dev->map, DW_IC_DATA_CMD, &tmp); - /* Ensure length byte is a valid value */ - if (flags & I2C_M_RECV_LEN && - tmp <= I2C_SMBUS_BLOCK_MAX && tmp > 0) { - len = i2c_dw_recv_len(dev, tmp); + if (flags & I2C_M_RECV_LEN) { + /* Ensure length byte is a valid value */ + if (tmp <= I2C_SMBUS_BLOCK_MAX && tmp > 0) + len = i2c_dw_recv_len(dev, tmp); + msgs[dev->msg_read_idx].flags &= ~I2C_M_RECV_LEN; } *buf++ = tmp; dev->rx_outstanding--;-- 2.27.0
-- With Best Regards, Andy Shevchenko