Commit b998a12e authored by Marek Roszko's avatar Marek Roszko Committed by Greg Kroah-Hartman

i2c: at91: add bound checking on SMBus block length bytes

commit 75b81f33 upstream.

The driver was not bound checking the received length byte to ensure it was within the
the buffer size that is allocated for SMBus blocks. This resulted in buffer overflows
whenever an invalid length byte was received.
It also failed to ensure the length byte was not zero. If it received zero, it would end up
in an infinite loop as the at91_twi_read_next_byte function returned immediately without
allowing RHR to be read to clear the RXRDY interrupt.

Tested agaisnt a SMBus compliant battery.
Signed-off-by: default avatarMarek Roszko <mark.roszko@gmail.com>
Acked-by: default avatarLudovic Desroches <ludovic.desroches@atmel.com>
Signed-off-by: default avatarWolfram Sang <wsa@the-dreams.de>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@linuxfoundation.org>
parent 1423ca3a
...@@ -101,6 +101,7 @@ struct at91_twi_dev { ...@@ -101,6 +101,7 @@ struct at91_twi_dev {
unsigned twi_cwgr_reg; unsigned twi_cwgr_reg;
struct at91_twi_pdata *pdata; struct at91_twi_pdata *pdata;
bool use_dma; bool use_dma;
bool recv_len_abort;
struct at91_twi_dma dma; struct at91_twi_dma dma;
}; };
...@@ -267,12 +268,24 @@ static void at91_twi_read_next_byte(struct at91_twi_dev *dev) ...@@ -267,12 +268,24 @@ static void at91_twi_read_next_byte(struct at91_twi_dev *dev)
*dev->buf = at91_twi_read(dev, AT91_TWI_RHR) & 0xff; *dev->buf = at91_twi_read(dev, AT91_TWI_RHR) & 0xff;
--dev->buf_len; --dev->buf_len;
/* return if aborting, we only needed to read RHR to clear RXRDY*/
if (dev->recv_len_abort)
return;
/* handle I2C_SMBUS_BLOCK_DATA */ /* handle I2C_SMBUS_BLOCK_DATA */
if (unlikely(dev->msg->flags & I2C_M_RECV_LEN)) { if (unlikely(dev->msg->flags & I2C_M_RECV_LEN)) {
dev->msg->flags &= ~I2C_M_RECV_LEN; /* ensure length byte is a valid value */
dev->buf_len += *dev->buf; if (*dev->buf <= I2C_SMBUS_BLOCK_MAX && *dev->buf > 0) {
dev->msg->len = dev->buf_len + 1; dev->msg->flags &= ~I2C_M_RECV_LEN;
dev_dbg(dev->dev, "received block length %d\n", dev->buf_len); dev->buf_len += *dev->buf;
dev->msg->len = dev->buf_len + 1;
dev_dbg(dev->dev, "received block length %d\n",
dev->buf_len);
} else {
/* abort and send the stop by reading one more byte */
dev->recv_len_abort = true;
dev->buf_len = 1;
}
} }
/* send stop if second but last byte has been read */ /* send stop if second but last byte has been read */
...@@ -444,6 +457,12 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev) ...@@ -444,6 +457,12 @@ static int at91_do_twi_transfer(struct at91_twi_dev *dev)
ret = -EIO; ret = -EIO;
goto error; goto error;
} }
if (dev->recv_len_abort) {
dev_err(dev->dev, "invalid smbus block length recvd\n");
ret = -EPROTO;
goto error;
}
dev_dbg(dev->dev, "transfer complete\n"); dev_dbg(dev->dev, "transfer complete\n");
return 0; return 0;
...@@ -500,6 +519,7 @@ static int at91_twi_xfer(struct i2c_adapter *adap, struct i2c_msg *msg, int num) ...@@ -500,6 +519,7 @@ static int at91_twi_xfer(struct i2c_adapter *adap, struct i2c_msg *msg, int num)
dev->buf_len = m_start->len; dev->buf_len = m_start->len;
dev->buf = m_start->buf; dev->buf = m_start->buf;
dev->msg = m_start; dev->msg = m_start;
dev->recv_len_abort = false;
ret = at91_do_twi_transfer(dev); ret = at91_do_twi_transfer(dev);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment