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

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

[Abdurrahman Hussain]

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>

[PATCH 1/5] hwmon: (pmbus/adm1266) seed timestamp from the real-time clock

[Abdurrahman Hussain]

adm1266_set_rtc() seeds the chip's SET_RTC register from
ktime_get_seconds(), which returns CLOCK_MONOTONIC -- i.e. seconds
since the host last booted, not seconds since the Unix epoch.

The chip stamps that value into every blackbox record it captures.
Userspace reading those timestamps back expects wall-clock seconds:
that's what the SET_RTC frame layout documents (datasheet Rev. D,
Table 84) and what every other consumer of "seconds since epoch"
assumes.  Seeding from CLOCK_MONOTONIC gives blackbox records a
timestamp that is only meaningful within a single boot of the host
and silently resets to small values on every reboot.

Switch to ktime_get_real_seconds() so the seed matches what the
register is documented to hold.

Fixes: 15609d189302 ("hwmon: (pmbus/adm1266) read blackbox")
Cc: stable@vger.kernel.org
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/hwmon/pmbus/adm1266.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index d90f8f80be8e..a86666c73a5e 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -432,7 +432,7 @@ static int adm1266_set_rtc(struct adm1266_data *data)
 	char write_buf[6];
 	int i;
 
-	kt = ktime_get_seconds();
+	kt = ktime_get_real_seconds();
 
 	memset(write_buf, 0, sizeof(write_buf));
 

-- 
2.53.0

[PATCH 2/5] hwmon: (pmbus/adm1266) widen blackbox-info buffer to I2C_SMBUS_BLOCK_MAX

[Abdurrahman Hussain]

adm1266_nvmem_read_blackbox() declares a 5-byte stack buffer and
passes it to i2c_smbus_read_block_data() to retrieve the 4-byte
BLACKBOX_INFO response.  i2c_smbus_read_block_data() does not honour
caller buffer sizes -- it memcpy()s data.block[0] bytes from the
SMBus transaction (where data.block[0] is the length byte returned by
the slave device, up to I2C_SMBUS_BLOCK_MAX = 32):

	memcpy(values, &data.block[1], data.block[0]);

If the device returns any block length above 5, the call overflows
the caller's 5-byte stack buffer before the post-call

	if (ret != 4)
		return -EIO;

check has a chance to reject the response.

Widen the local buffer to I2C_SMBUS_BLOCK_MAX so the helper has room
for any well-formed SMBus block response, matching the convention used
by the other i2c_smbus_read_block_data() callers in this driver.

Fixes: 15609d189302 ("hwmon: (pmbus/adm1266) read blackbox")
Cc: stable@vger.kernel.org
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/hwmon/pmbus/adm1266.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index a86666c73a5e..94691dec1359 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -349,7 +349,7 @@ static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)
 {
 	int record_count;
 	char index;
-	u8 buf[5];
+	u8 buf[I2C_SMBUS_BLOCK_MAX];
 	int ret;
 
 	ret = i2c_smbus_read_block_data(data->client, ADM1266_BLACKBOX_INFO, buf);

-- 
2.53.0

[PATCH 3/5] hwmon: (pmbus/adm1266) reject implausible blackbox record_count

[Abdurrahman Hussain]

adm1266_nvmem_read_blackbox() loops over a record_count that comes
straight from byte 3 of the BLACKBOX_INFO response.  The destination
buffer is data->dev_mem, sized for the nvmem cell's declared 2048
bytes (ADM1266_BLACKBOX_MAX_RECORDS * ADM1266_BLACKBOX_SIZE = 32 * 64).
A device that reports a record_count greater than 32 -- whether due
to firmware bugs, bus corruption, or a non-responsive slave returning
0xff -- would walk read_buff past the end of the dev_mem allocation
on the trailing iterations.

Cap record_count at ADM1266_BLACKBOX_MAX_RECORDS (introduced here)
before entering the loop and return -EIO on any larger value, so a
malformed BLACKBOX_INFO response cannot drive the loop out of bounds.

Fixes: 15609d189302 ("hwmon: (pmbus/adm1266) read blackbox")
Cc: stable@vger.kernel.org
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/hwmon/pmbus/adm1266.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index 94691dec1359..43d9e7407795 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -46,6 +46,7 @@
 
 #define ADM1266_BLACKBOX_OFFSET		0
 #define ADM1266_BLACKBOX_SIZE		64
+#define ADM1266_BLACKBOX_MAX_RECORDS	32
 
 #define ADM1266_PMBUS_BLOCK_MAX		255
 
@@ -360,6 +361,8 @@ static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)
 		return -EIO;
 
 	record_count = buf[3];
+	if (record_count > ADM1266_BLACKBOX_MAX_RECORDS)
+		return -EIO;
 
 	for (index = 0; index < record_count; index++) {
 		ret = adm1266_pmbus_block_xfer(data, ADM1266_READ_BLACKBOX, 1, &index, read_buff);

-- 
2.53.0

[PATCH 4/5] hwmon: (pmbus/adm1266) include PEC byte in pmbus_block_xfer read buffer

[Abdurrahman Hussain]

adm1266_pmbus_block_xfer() sets up the read transaction with

	.buf = data->read_buf,
	.len = ADM1266_PMBUS_BLOCK_MAX + 2,

but read_buf in struct adm1266_data is declared as

	u8 read_buf[ADM1266_PMBUS_BLOCK_MAX + 1];

For a max-length block response (length byte = 255 + up to 1 PEC
byte), the i2c controller is told to write 257 bytes into a 256-byte
buffer, putting one byte past the end of read_buf.  The same response
also makes the subsequent PEC compare

	if (crc != msgs[1].buf[msgs[1].buf[0] + 1])

read a byte beyond the array.

Bump the read_buf declaration to ADM1266_PMBUS_BLOCK_MAX + 2 so the
buffer can hold the length byte, up to 255 payload bytes, and the PEC
byte the i2c_msg length already accounts for.

Fixes: 407dc802a9c0 ("hwmon: (pmbus/adm1266) Add Block process call")
Cc: stable@vger.kernel.org
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/hwmon/pmbus/adm1266.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index 43d9e7407795..5c68e3177f64 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -61,7 +61,7 @@ struct adm1266_data {
 	u8 *dev_mem;
 	struct mutex buf_mutex;
 	u8 write_buf[ADM1266_PMBUS_BLOCK_MAX + 1] ____cacheline_aligned;
-	u8 read_buf[ADM1266_PMBUS_BLOCK_MAX + 1] ____cacheline_aligned;
+	u8 read_buf[ADM1266_PMBUS_BLOCK_MAX + 2] ____cacheline_aligned;
 };
 
 static const struct nvmem_cell_info adm1266_nvmem_cells[] = {

-- 
2.53.0

[PATCH 5/5] hwmon: (pmbus/adm1266) bounce blackbox records through a protocol-sized buffer

[Abdurrahman Hussain]

adm1266_pmbus_block_xfer() copies the device-supplied block payload
into the caller-provided buffer using the device-supplied length:

	memcpy(data_r, &msgs[1].buf[1], msgs[1].buf[0]);

The helper does not know how large data_r is and trusts the device to
return at most one record's worth of bytes.  adm1266_nvmem_read_blackbox()
violates that contract: it advances read_buff inside data->dev_mem in
ADM1266_BLACKBOX_SIZE (64-byte) strides while the helper is willing to
write up to ADM1266_PMBUS_BLOCK_MAX (255) bytes.  A device that returns
more than 64 bytes on the trailing record (read_buff offset 1984 in
the 2048-byte dev_mem allocation) overflows dev_mem by up to 191 bytes
before the post-call

	if (ret != ADM1266_BLACKBOX_SIZE)
		return -EIO;

can reject the response.

Contain the fix in the caller without changing the helper signature:
read each record into a 255-byte local bounce buffer that matches the
helper's maximum output, validate the returned length, and only then
copy exactly ADM1266_BLACKBOX_SIZE bytes into the dev_mem slot.

Fixes: 407dc802a9c0 ("hwmon: (pmbus/adm1266) Add Block process call")
Cc: stable@vger.kernel.org
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/hwmon/pmbus/adm1266.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index 5c68e3177f64..ea1edb89d2a0 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -348,6 +348,7 @@ static void adm1266_init_debugfs(struct adm1266_data *data)
 
 static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)
 {
+	u8 record[ADM1266_PMBUS_BLOCK_MAX];
 	int record_count;
 	char index;
 	u8 buf[I2C_SMBUS_BLOCK_MAX];
@@ -365,13 +366,14 @@ static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)
 		return -EIO;
 
 	for (index = 0; index < record_count; index++) {
-		ret = adm1266_pmbus_block_xfer(data, ADM1266_READ_BLACKBOX, 1, &index, read_buff);
+		ret = adm1266_pmbus_block_xfer(data, ADM1266_READ_BLACKBOX, 1, &index, record);
 		if (ret < 0)
 			return ret;
 
 		if (ret != ADM1266_BLACKBOX_SIZE)
 			return -EIO;
 
+		memcpy(read_buff, record, ADM1266_BLACKBOX_SIZE);
 		read_buff += ADM1266_BLACKBOX_SIZE;
 	}
 

-- 
2.53.0

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

[Guenter Roeck]

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

[Abdurrahman Hussain]

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