| Message ID | 20181115192446.17187-5-minyard@acm.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | RFC: Fix/add vmstate handling in some I2C code | expand |
On 15 November 2018 at 19:24, <minyard@acm.org> wrote: > From: Corey Minyard <cminyard@mvista.com> > > Avoid an overflow. > > Signed-off-by: Corey Minyard <cminyard@mvista.com> > --- > hw/i2c/smbus_slave.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/hw/i2c/smbus_slave.c b/hw/i2c/smbus_slave.c > index 83ca041b5d..fa988919d8 100644 > --- a/hw/i2c/smbus_slave.c > +++ b/hw/i2c/smbus_slave.c > @@ -182,7 +182,11 @@ static int smbus_i2c_send(I2CSlave *s, uint8_t data) > switch (dev->mode) { > case SMBUS_WRITE_DATA: > DPRINTF("Write data %02x\n", data); > - dev->data_buf[dev->data_len++] = data; > + if (dev->data_len >= sizeof(dev->data_buf)) { > + BADF("Too many bytes sent\n"); > + } else { > + dev->data_buf[dev->data_len++] = data; > + } > break; Reviewed-by: Peter Maydell <peter.maydell@linaro.org> What happens on a real device in this situation ? thanks -- PMM
On 11/20/18 9:33 AM, Peter Maydell wrote: > On 15 November 2018 at 19:24, <minyard@acm.org> wrote: >> From: Corey Minyard <cminyard@mvista.com> >> >> Avoid an overflow. >> >> Signed-off-by: Corey Minyard <cminyard@mvista.com> >> --- >> hw/i2c/smbus_slave.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/hw/i2c/smbus_slave.c b/hw/i2c/smbus_slave.c >> index 83ca041b5d..fa988919d8 100644 >> --- a/hw/i2c/smbus_slave.c >> +++ b/hw/i2c/smbus_slave.c >> @@ -182,7 +182,11 @@ static int smbus_i2c_send(I2CSlave *s, uint8_t data) >> switch (dev->mode) { >> case SMBUS_WRITE_DATA: >> DPRINTF("Write data %02x\n", data); >> - dev->data_buf[dev->data_len++] = data; >> + if (dev->data_len >= sizeof(dev->data_buf)) { >> + BADF("Too many bytes sent\n"); >> + } else { >> + dev->data_buf[dev->data_len++] = data; >> + } >> break; > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > What happens on a real device in this situation ? It's device specific. Some devices (most eeproms, I suspect) will just keep taking data, wrapping around when you hit the end of the memory. Some devices (IPMI BMCs, generally) will ignore the extra data. Some devices may return a NAK on a byte if it gets too much data. The specification says: The slave device detects an invalid command or invalid data. In this case the slave device must NACK the received byte. The master upon detection of this condition must generate a STOP condition and retry the transaction. So a NAK may be appropriate here, but it's kind of fuzzy. Since generating a NAK is more complicated, I would guess that most devices don't. -corey > thanks > -- PMM
diff --git a/hw/i2c/smbus_slave.c b/hw/i2c/smbus_slave.c index 83ca041b5d..fa988919d8 100644 --- a/hw/i2c/smbus_slave.c +++ b/hw/i2c/smbus_slave.c @@ -182,7 +182,11 @@ static int smbus_i2c_send(I2CSlave *s, uint8_t data) switch (dev->mode) { case SMBUS_WRITE_DATA: DPRINTF("Write data %02x\n", data); - dev->data_buf[dev->data_len++] = data; + if (dev->data_len >= sizeof(dev->data_buf)) { + BADF("Too many bytes sent\n"); + } else { + dev->data_buf[dev->data_len++] = data; + } break; default: