[PATCH 0/5] hwmon: (pmbus/adm1266) buffer-bound and timestamp fixes

Abdurrahman Hussain posted 5 patches 1 week, 2 days ago
drivers/hwmon/pmbus/adm1266.c | 13 +++++++++----
1 file changed, 9 insertions(+), 4 deletions(-)
[PATCH 0/5] hwmon: (pmbus/adm1266) buffer-bound and timestamp fixes
Posted by Abdurrahman Hussain 1 week, 2 days ago
This series fixes five pre-existing bugs in adm1266.c that were
surfaced by automated review of an in-flight feature series for the
same driver [1].  None of them are introduced by that feature work --
they are all reachable on the existing driver as it sits in mainline.
Sending them standalone first, with Fixes: tags and Cc: stable, so
the feature respin (v5) can rebase on top.

Patch 1 fixes a CLOCK_MONOTONIC vs CLOCK_REALTIME confusion in
adm1266_set_rtc(): the chip's SET_RTC register is documented to hold
wall-clock seconds, but the driver currently seeds it from
ktime_get_seconds(), giving blackbox records timestamps that reset
to small values on every host reboot.

Patches 2 and 3 fix two ways the blackbox-info path can be driven
out of bounds by a misbehaving slave: a 5-byte stack buffer that
i2c_smbus_read_block_data() will memcpy() up to 32 bytes into, and
a record_count loop bound taken directly from the device with no
upper clamp against the 32-record dev_mem allocation.

Patches 4 and 5 fix the two ways adm1266_pmbus_block_xfer() can
write past the end of a buffer: an off-by-one on the helper's own
read_buf (sized for the length+payload but missing the PEC byte the
i2c_msg length already accounts for), and a caller-side bug where
adm1266_nvmem_read_blackbox() advances its destination pointer in
64-byte strides while the helper is willing to write up to 255
bytes per call.

[1] https://lore.kernel.org/r/20260512-adm1266-v3-0-a81a479b0bb0@nexthop.ai

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
Abdurrahman Hussain (5):
      hwmon: (pmbus/adm1266) seed timestamp from the real-time clock
      hwmon: (pmbus/adm1266) widen blackbox-info buffer to I2C_SMBUS_BLOCK_MAX
      hwmon: (pmbus/adm1266) reject implausible blackbox record_count
      hwmon: (pmbus/adm1266) include PEC byte in pmbus_block_xfer read buffer
      hwmon: (pmbus/adm1266) bounce blackbox records through a protocol-sized buffer

 drivers/hwmon/pmbus/adm1266.c | 13 +++++++++----
 1 file changed, 9 insertions(+), 4 deletions(-)
---
base-commit: 1f63dd8ca0dc05a8272bb8155f643c691d29bb11
change-id: 20260514-adm1266-fixes-853003a0fad4

Best regards,
--  
Abdurrahman Hussain <abdurrahman@nexthop.ai>
Re: [PATCH 0/5] hwmon: (pmbus/adm1266) buffer-bound and timestamp fixes
Posted by Guenter Roeck 1 week, 1 day ago
On 5/15/26 15:11, Abdurrahman Hussain wrote:
> This series fixes five pre-existing bugs in adm1266.c that were
> surfaced by automated review of an in-flight feature series for the
> same driver [1].  None of them are introduced by that feature work --
> they are all reachable on the existing driver as it sits in mainline.
> Sending them standalone first, with Fixes: tags and Cc: stable, so
> the feature respin (v5) can rebase on top.
> 
> Patch 1 fixes a CLOCK_MONOTONIC vs CLOCK_REALTIME confusion in
> adm1266_set_rtc(): the chip's SET_RTC register is documented to hold
> wall-clock seconds, but the driver currently seeds it from
> ktime_get_seconds(), giving blackbox records timestamps that reset
> to small values on every host reboot.
> 
> Patches 2 and 3 fix two ways the blackbox-info path can be driven
> out of bounds by a misbehaving slave: a 5-byte stack buffer that
> i2c_smbus_read_block_data() will memcpy() up to 32 bytes into, and
> a record_count loop bound taken directly from the device with no
> upper clamp against the 32-record dev_mem allocation.
> 
> Patches 4 and 5 fix the two ways adm1266_pmbus_block_xfer() can
> write past the end of a buffer: an off-by-one on the helper's own
> read_buf (sized for the length+payload but missing the PEC byte the
> i2c_msg length already accounts for), and a caller-side bug where
> adm1266_nvmem_read_blackbox() advances its destination pointer in
> 64-byte strides while the helper is willing to write up to 255
> bytes per call.
> 
> [1] https://lore.kernel.org/r/20260512-adm1266-v3-0-a81a479b0bb0@nexthop.ai
> 
> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
> ---
> Abdurrahman Hussain (5):
>        hwmon: (pmbus/adm1266) seed timestamp from the real-time clock
>        hwmon: (pmbus/adm1266) widen blackbox-info buffer to I2C_SMBUS_BLOCK_MAX
>        hwmon: (pmbus/adm1266) reject implausible blackbox record_count
>        hwmon: (pmbus/adm1266) include PEC byte in pmbus_block_xfer read buffer
>        hwmon: (pmbus/adm1266) bounce blackbox records through a protocol-sized buffer
> 
>   drivers/hwmon/pmbus/adm1266.c | 13 +++++++++----
>   1 file changed, 9 insertions(+), 4 deletions(-)
> ---
> base-commit: 1f63dd8ca0dc05a8272bb8155f643c691d29bb11
> change-id: 20260514-adm1266-fixes-853003a0fad4
> 
> Best regards,
> --
> Abdurrahman Hussain <abdurrahman@nexthop.ai>
> 

Sashiko identified several issues with the driver as part of the review.
Most if not all of them seem valid, but were not introduced with this
series. I'll apply the series as is. Any fixes can come later.

Thanks,
Guenter
Re: [PATCH 0/5] hwmon: (pmbus/adm1266) buffer-bound and timestamp fixes
Posted by Abdurrahman Hussain 1 week, 1 day ago
On Sat May 16, 2026 at 8:23 AM PDT, Guenter Roeck wrote:
> On 5/15/26 15:11, Abdurrahman Hussain wrote:
>> This series fixes five pre-existing bugs in adm1266.c that were
>> surfaced by automated review of an in-flight feature series for the
>> same driver [1].  None of them are introduced by that feature work --
>> they are all reachable on the existing driver as it sits in mainline.
>> Sending them standalone first, with Fixes: tags and Cc: stable, so
>> the feature respin (v5) can rebase on top.
>> 
>> Patch 1 fixes a CLOCK_MONOTONIC vs CLOCK_REALTIME confusion in
>> adm1266_set_rtc(): the chip's SET_RTC register is documented to hold
>> wall-clock seconds, but the driver currently seeds it from
>> ktime_get_seconds(), giving blackbox records timestamps that reset
>> to small values on every host reboot.
>> 
>> Patches 2 and 3 fix two ways the blackbox-info path can be driven
>> out of bounds by a misbehaving slave: a 5-byte stack buffer that
>> i2c_smbus_read_block_data() will memcpy() up to 32 bytes into, and
>> a record_count loop bound taken directly from the device with no
>> upper clamp against the 32-record dev_mem allocation.
>> 
>> Patches 4 and 5 fix the two ways adm1266_pmbus_block_xfer() can
>> write past the end of a buffer: an off-by-one on the helper's own
>> read_buf (sized for the length+payload but missing the PEC byte the
>> i2c_msg length already accounts for), and a caller-side bug where
>> adm1266_nvmem_read_blackbox() advances its destination pointer in
>> 64-byte strides while the helper is willing to write up to 255
>> bytes per call.
>> 
>> [1] https://lore.kernel.org/r/20260512-adm1266-v3-0-a81a479b0bb0@nexthop.ai
>> 
>> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
>> ---
>> Abdurrahman Hussain (5):
>>        hwmon: (pmbus/adm1266) seed timestamp from the real-time clock
>>        hwmon: (pmbus/adm1266) widen blackbox-info buffer to I2C_SMBUS_BLOCK_MAX
>>        hwmon: (pmbus/adm1266) reject implausible blackbox record_count
>>        hwmon: (pmbus/adm1266) include PEC byte in pmbus_block_xfer read buffer
>>        hwmon: (pmbus/adm1266) bounce blackbox records through a protocol-sized buffer
>> 
>>   drivers/hwmon/pmbus/adm1266.c | 13 +++++++++----
>>   1 file changed, 9 insertions(+), 4 deletions(-)
>> ---
>> base-commit: 1f63dd8ca0dc05a8272bb8155f643c691d29bb11
>> change-id: 20260514-adm1266-fixes-853003a0fad4
>> 
>> Best regards,
>> --
>> Abdurrahman Hussain <abdurrahman@nexthop.ai>
>> 
>
> Sashiko identified several issues with the driver as part of the review.
> Most if not all of them seem valid, but were not introduced with this
> series. I'll apply the series as is. Any fixes can come later.
>
> Thanks,
> Guenter

Thanks Guenter!

I will address the issues in a follow up series.

Best regards,
Abdurrahman