Re: [Qemu-devel] [PATCH] smbus_eeprom: Limit data writes to 255 bytes

[Michael Hanselmann]

Hi Paolo

On 28.12.18 14:52, Paolo Bonzini wrote:
> On 27/12/18 12:51, Michael Hanselmann wrote:
>> The "eeprom_write_data" function in "smbus_eeprom.c" had no provisions
>> to limit the length of data written. If a caller were able to manipulate
>> the "len" parameter they could potentially write before or after the
>> target buffer.
>> ---
>>  hw/i2c/smbus_eeprom.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/hw/i2c/smbus_eeprom.c b/hw/i2c/smbus_eeprom.c
>> index f18aa3de35..74fa1c328c 100644
>> --- a/hw/i2c/smbus_eeprom.c
>> +++ b/hw/i2c/smbus_eeprom.c
>> @@ -76,6 +76,7 @@ static void eeprom_write_data(SMBusDevice *dev, uint8_t 
>> cmd, uint8_t *buf, int l
>>         It is a block write without a length byte.  Fortunately we
>>         get the full block anyway.  */
>>      /* TODO: Should this set the current location?  */
>> +    len &= 0xff;
>>      if (cmd + len > 256)
>>          n = 256 - cmd;
>>      else
>>
> 
> Note that len is limited to 33 bytes (smbus_do_write and smbus_i2c_send).

In practice it turns out to be the case. I thought I had discovered an
out-of-bounds write because hw/i2c/smbus.c:smbus_i2c_recv increases
dev->data_len unconditionally. The I2C controller implemented in
hw/i2c/aspeed_i2c.c and used by certain ARM board emulations allows
fine-grained control of the communication which allowed me to increase
data_len easily (up to and beyond an overflow if intended). It was only
the state machine in smbus.c which made it impossible to actually get to
a usable point in my experiment (increasing data_len requires
SMBUS_WRITE_DATA->SMBUS_READ_DATA, then the communication must be
stopped via NACK to avoid resetting data_len in I2C_FINISH, but there's
no way from SMBUS_DONE to SMBUS_WRITE_DATA).

Adding bitwise-and for 0xff defuses this particular situation regardless
of what state an attacker can bring the emulated devices into.

Best regards,
Michael

signature.asc
Description: OpenPGP digital signature