Re: PMBus memory overflow

Guenter Roeck
2025-04-20T03:03:03Z

On 4/19/25 19:29, Matt Corallo wrote:
>
>
> On 4/19/25 6:49 PM, Guenter Roeck wrote:
>> On 4/19/25 15:38, Guenter Roeck wrote:
>>> On 4/19/25 12:29, Matt Corallo wrote:
>>>
>>>> Hmm, doesn't seem to trigger it at least with pmbus_peek.c, the following diff still doesn't hit the too big prints:
>>>>
>>>
>>> Only idea I have at this point is to either enable smbus tracing in the kernel
>>> or (better) to add test code into i2c_smbus_read_block_data() to see what
>>> exactly triggers the problem.
>>>
>>
>> You could try this:
>>
>> diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c
>> index e73afbefe222..b2856f88431d 100644
>> --- a/drivers/i2c/i2c-core-smbus.c
>> +++ b/drivers/i2c/i2c-core-smbus.c
>> @@ -233,6 +233,9 @@ s32 i2c_smbus_read_block_data(const struct i2c_client *client, u8 command,
>>          if (status)
>>                  return status;
>>
>> +       if (data.block[0] > I2C_SMBUS_BLOCK_MAX)
>> +               return -E2BIG;
>> +
>>          memcpy(values, &data.block[1], data.block[0]);
>>          return data.block[0];
>>
>> and maybe temporarily dump the entire buffer if that is seen.
>>
>> diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c
>> index e73afbefe222..3dcb8b6b2427 100644
>> --- a/drivers/i2c/i2c-core-smbus.c
>> +++ b/drivers/i2c/i2c-core-smbus.c
>> @@ -233,6 +233,12 @@ s32 i2c_smbus_read_block_data(const struct i2c_client *client, u8 command,
>>          if (status)
>>                  return status;
>>
>> +       if (data.block[0] > I2C_SMBUS_BLOCK_MAX) {
>> +               print_hex_dump_bytes("Bad SMBus block data: ", DUMP_PREFIX_NONE,
>> +                                    data.block, I2C_SMBUS_BLOCK_MAX);
>> +               return -E2BIG;
>> +       }
>> +
>>          memcpy(values, &data.block[1], data.block[0]);
>>          return data.block[0];
>>   }
>>
>> If that doesn't trigger I am going to be really puzzled.
>
> With this patch the device works fine. With debug enabled I see:
>
> [  273.730583] i2c-core: driver [pmbus] registered
> [  276.585423] pmbus 1-0058: probe
> [  278.925878] Bad SMBus block data: ff ff b8 ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> [  278.925881] Bad SMBus block data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> [  278.937859] Bad SMBus block data: ff ff da ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> [  278.937861] Bad SMBus block data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff  ................
> [  280.353885] i2c i2c-1: client [pmbus] registered with bus id 1-0058
> [  280.353888] i2c i2c-1: new_device: Instantiated device pmbus at 0x58

Great, thanks for confirming. So the two problems are:

- The cp2112 driver does not validate the first data byte (the length field) when
receiving SMBus block data from connected devices.
- The I2C (SMBus) core does not validate the first data byte before copying
the data buffer.

Copying the I2C subsystem mailing list and the I2C maintainer.

Wolfram, what do you suggest ? Fixing the cp2112 driver is obviously necessary, but
I do wonder if a check such as the one above would be appropriate as well, possibly
even combined with a WARN_ONCE().

Thanks,
Guenter

> matt@psu-kern:~$ sensors
> pmbus-i2c-1-58
> Adapter: CP2112 SMBus Bridge on hidraw1
> vin:         123.00 V  (crit min =  -0.50 V, min =  -0.50 V)  ALARM (MIN)
>                        (max =  -0.50 V, crit max =  -0.50 V)
> vcap:        -500.00 mV
> vout1:        12.05 V  (crit min = +128.00 V, min = +128.00 V)
>                        (max = +128.00 V, crit max = +128.00 V)
> fan1:        5304 RPM
> fan2:          -1 RPM
> fan3:           FAULT  ALARM
> fan4:           FAULT  ALARM
> temp1:        +27.0°C  (low  =  -0.5°C, high =  -0.5°C)
>                        (crit low =  -0.5°C, crit =  -0.5°C)
> temp2:        +32.0°C  (low  =  -0.5°C, high =  -0.5°C)
>                        (crit low =  -0.5°C, crit =  -0.5°C)
> temp3:         -0.5°C  (low  =  -0.5°C, high =  -0.5°C)
>                        (crit low =  -0.5°C, crit =  -0.5°C)
> pin:         102.00 W  (max =   1.60 kW)
> pout1:        88.00 W  (max =   0.00 W, crit = -500.00 mW)
>                        (cap = -500.00 mW)
> iin:         988.00 mA (max =  -0.01 A, crit max =  -0.50 A)
> iout1:         7.39 A  (crit min =  -0.50 A, max =  -0.50 A)
>                        (crit max =  -0.50 A)