[PATCH v3 0/5] hwmon: (pmbus/adm1266) blackbox, FW rev, rtc_class, GPIO label

[PATCH v3 0/5] hwmon: (pmbus/adm1266) blackbox, FW rev, rtc_class, GPIO label

[Abdurrahman Hussain]

This series fills in some gaps in the adm1266 driver and removes a
probe-time RTC seed that was clobbering valid device state.

Patch 1 exposes the firmware revision (IC_DEVICE_REV, 0xAE) via
debugfs alongside the existing sequencer_state entry.

Patch 2 adds a write-only clear_blackbox debugfs file. Devices
configured for single-recording mode (BLACKBOX_CONFIG[0] = 0) need
an explicit clear once the 32-record buffer fills; the documented
sub-command ({0xFE, 0x00} block-write to 0xDE) wasn't reachable
from userspace.

Patch 3 exposes the non-volatile POWERUP_COUNTER (0xE4) via debugfs.
The same value is embedded in every blackbox record, so the live
value lets userspace match a captured record back to the boot it
came from when correlating logs.

Patch 4 replaces the probe-time SET_RTC seed with an rtc_class
device backed by SET_RTC. The existing seed used CLOCK_MONOTONIC
seconds, which is meaningless as a wall-clock time and clobbers
whatever the device retained across a warm reboot. The data sheet
(Rev. D, p. 22) explicitly recommends "frequently send the time
stamp to the ADM1266 to synchronize the UNIX time and reduce the
time from drifting" when running on the internal oscillator (no
external 32.768 kHz crystal). Letting userspace own the policy via
standard tooling (hwclock, chrony, systemd-timesyncd) against
/dev/rtcN is both more correct and avoids any driver-specific
sysfs ABI.

Patch 5 disambiguates GPIO line labels on platforms that fit two
ADM1266 devices on different I2C buses at the same slave address.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
Changes in v3:
- Patch 2: take guard(pmbus_lock)(client) in
  adm1266_clear_blackbox_write() and adm1266_nvmem_read_blackbox()
  to serialise the clear against the multi-record nvmem read loop
  (both share command 0xDE). Also move adm1266_config_nvmem() to
  after pmbus_do_probe() so the blackbox nvmem entry isn't exposed
  to userspace before the PMBus mutex is initialised (Sashiko
  review).
- Patch 4: take guard(pmbus_lock)(client) in the RTC
  read_time/set_time callbacks to serialise against the PMBus
  core's PAGE+register sequences. Move adm1266_register_rtc() to
  after pmbus_do_probe() for the same reason as the config_nvmem
  move; /dev/rtcN being opened by userspace (systemd-timesyncd,
  udev) before probe completed would otherwise hit an
  uninitialised mutex (observed on hardware, also flagged by
  Sashiko).
- Link to v2: https://patch.msgid.link/20260510-adm1266-v2-0-3a22b903c2f1@nexthop.ai

Changes in v2:
- Drop the v1 "use wall-clock seconds for SET_RTC" and "write
  fractional-seconds field of SET_RTC" patches. v1 patch 6 already
  added an rtc_class device, so userspace now owns the time-set
  policy and the probe-time seed is no longer needed - removing it
  also fixes the cross-reboot blackbox-correlation regression the
  seed introduced.
- In the rtc_class patch, write the SET_RTC fractional-seconds
  bytes as zero rather than computing them from a timespec64. The
  rtc_class API is second-precision, so the divide that v1 patch 2
  was adding never produced a non-zero result anyway. This also
  drops the 64-bit divide that would have failed to link on 32-bit
  builds (Sashiko review).
- Use %d (decimal) for the I2C adapter number in GPIO line labels,
  matching the i2c-N convention used elsewhere in Linux. v1 used
  %x, which rendered bus numbers >= 10 in hexadecimal (Sashiko
  review).
- Link to v1: https://patch.msgid.link/20260508-adm1266-v1-0-ec08bf29e0ce@nexthop.ai

To: Guenter Roeck <linux@roeck-us.net>
To: Alexandru Tachici <alexandru.tachici@analog.com>
Cc: linux-hwmon@vger.kernel.org
Cc: linux-kernel@vger.kernel.org

---
Abdurrahman Hussain (5):
      hwmon: (pmbus/adm1266) add firmware_revision debugfs entry
      hwmon: (pmbus/adm1266) add clear_blackbox debugfs entry
      hwmon: (pmbus/adm1266) add powerup_counter debugfs entry
      hwmon: (pmbus/adm1266) replace probe-time RTC seed with rtc_class device
      hwmon: (pmbus/adm1266) include adapter number in GPIO line label

 drivers/hwmon/pmbus/adm1266.c | 178 ++++++++++++++++++++++++++++++++++++++----
 1 file changed, 161 insertions(+), 17 deletions(-)
---
base-commit: 917719c412c48687d4a176965d1fa35320ec457c
change-id: 20260507-adm1266-cf3af42dc3d2

Best regards,
--  
Abdurrahman Hussain <abdurrahman@nexthop.ai>

[PATCH v3 1/5] hwmon: (pmbus/adm1266) add firmware_revision debugfs entry

[Abdurrahman Hussain]

The ADM1266 reports its firmware revision via the IC_DEVICE_REV
manufacturer-specific block-read command (0xAE, datasheet Rev. D
Table 80). The first three returned bytes are the firmware
major.minor.patch fields. This is useful when correlating field
behaviour against ADI release notes; expose it through debugfs
alongside the existing sequencer_state entry.

The standard PMBus MFR_REVISION (0x9B) register is already exposed
by pmbus_core's debugfs auto-create path and reports the
manufacturer revision, which is a separate thing from the firmware
running on the device.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/hwmon/pmbus/adm1266.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index d90f8f80be8e..7b8433226176 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -21,6 +21,7 @@
 #include <linux/slab.h>
 #include <linux/timekeeping.h>
 
+#define ADM1266_IC_DEVICE_REV	0xAE
 #define ADM1266_BLACKBOX_CONFIG	0xD3
 #define ADM1266_PDIO_CONFIG	0xD4
 #define ADM1266_READ_STATE	0xD9
@@ -331,6 +332,30 @@ static int adm1266_state_read(struct seq_file *s, void *pdata)
 	return 0;
 }
 
+/*
+ * IC_DEVICE_REV (0xAE) returns an 8-byte block (datasheet Rev. D, Table 80):
+ *   [2:0] firmware revision  major.minor.patch
+ *   [5:3] bootloader revision major.minor.patch
+ *   [7:6] silicon revision    two ASCII characters
+ */
+static int adm1266_firmware_revision_read(struct seq_file *s, void *pdata)
+{
+	struct device *dev = s->private;
+	struct i2c_client *client = to_i2c_client(dev);
+	u8 buf[I2C_SMBUS_BLOCK_MAX];
+	int ret;
+
+	ret = i2c_smbus_read_block_data(client, ADM1266_IC_DEVICE_REV, buf);
+	if (ret < 0)
+		return ret;
+	if (ret < 3)
+		return -EIO;
+
+	seq_printf(s, "%u.%u.%u\n", buf[0], buf[1], buf[2]);
+
+	return 0;
+}
+
 static void adm1266_init_debugfs(struct adm1266_data *data)
 {
 	struct dentry *root;
@@ -343,6 +368,8 @@ static void adm1266_init_debugfs(struct adm1266_data *data)
 
 	debugfs_create_devm_seqfile(&data->client->dev, "sequencer_state", data->debugfs_dir,
 				    adm1266_state_read);
+	debugfs_create_devm_seqfile(&data->client->dev, "firmware_revision", data->debugfs_dir,
+				    adm1266_firmware_revision_read);
 }
 
 static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)

-- 
2.53.0

[PATCH v3 2/5] hwmon: (pmbus/adm1266) add clear_blackbox debugfs entry

[Abdurrahman Hussain]

The ADM1266 blackbox can be configured in two recording modes via
BLACKBOX_CONFIG[0]: cyclic, where the device overwrites the oldest
record once the 32-record buffer fills, and single, where it stops
recording until the buffer is cleared. Deployments that need to
preserve the full record history across multiple fault episodes
typically run in single mode and need a way to clear the buffer
after the records have been collected.

Expose a write-only debugfs file alongside sequencer_state. Writing
any data to it issues the documented clear-blackbox sub-command:
a 2-byte block-write to READ_BLACKBOX (0xDE) with payload
{0xFE, 0x00} (datasheet Rev. D).

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/hwmon/pmbus/adm1266.c | 48 +++++++++++++++++++++++++++++++++++++------
 1 file changed, 42 insertions(+), 6 deletions(-)

diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index 7b8433226176..bd3784d59a7c 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -356,6 +356,40 @@ static int adm1266_firmware_revision_read(struct seq_file *s, void *pdata)
 	return 0;
 }
 
+/*
+ * Clearing the blackbox is required when the device is configured in
+ * single-recording mode (BLACKBOX_CONFIG[0] = 0): once the 32-record
+ * buffer is full the device stops recording until cleared.
+ *
+ * The clear is issued as a 2-byte block-write to READ_BLACKBOX with
+ * payload {0xFE, 0x00} per the datasheet. READ_BLACKBOX is also used
+ * by adm1266_nvmem_read_blackbox() to walk records one at a time, so
+ * both paths take pmbus_lock to keep the clear from interleaving
+ * mid-iteration and corrupting the read sequence.
+ */
+static ssize_t adm1266_clear_blackbox_write(struct file *file, const char __user *ubuf,
+					    size_t count, loff_t *ppos)
+{
+	struct i2c_client *client = file->private_data;
+	u8 payload[2] = { 0xFE, 0x00 };
+	int ret;
+
+	guard(pmbus_lock)(client);
+	ret = i2c_smbus_write_block_data(client, ADM1266_READ_BLACKBOX,
+					 sizeof(payload), payload);
+	if (ret < 0)
+		return ret;
+
+	return count;
+}
+
+static const struct file_operations adm1266_clear_blackbox_fops = {
+	.owner = THIS_MODULE,
+	.open = simple_open,
+	.write = adm1266_clear_blackbox_write,
+	.llseek = noop_llseek,
+};
+
 static void adm1266_init_debugfs(struct adm1266_data *data)
 {
 	struct dentry *root;
@@ -370,6 +404,8 @@ static void adm1266_init_debugfs(struct adm1266_data *data)
 				    adm1266_state_read);
 	debugfs_create_devm_seqfile(&data->client->dev, "firmware_revision", data->debugfs_dir,
 				    adm1266_firmware_revision_read);
+	debugfs_create_file("clear_blackbox", 0200, data->debugfs_dir, data->client,
+			    &adm1266_clear_blackbox_fops);
 }
 
 static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)
@@ -379,10 +415,11 @@ static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)
 	u8 buf[5];
 	int ret;
 
+	guard(pmbus_lock)(data->client);
+
 	ret = i2c_smbus_read_block_data(data->client, ADM1266_BLACKBOX_INFO, buf);
 	if (ret < 0)
 		return ret;
-
 	if (ret != 4)
 		return -EIO;
 
@@ -392,7 +429,6 @@ static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)
 		ret = adm1266_pmbus_block_xfer(data, ADM1266_READ_BLACKBOX, 1, &index, read_buff);
 		if (ret < 0)
 			return ret;
-
 		if (ret != ADM1266_BLACKBOX_SIZE)
 			return -EIO;
 
@@ -497,14 +533,14 @@ static int adm1266_probe(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
-	ret = adm1266_config_nvmem(data);
-	if (ret < 0)
-		return ret;
-
 	ret = pmbus_do_probe(client, &data->info);
 	if (ret)
 		return ret;
 
+	ret = adm1266_config_nvmem(data);
+	if (ret < 0)
+		return ret;
+
 	adm1266_init_debugfs(data);
 
 	return 0;

-- 
2.53.0

[PATCH v3 3/5] hwmon: (pmbus/adm1266) add powerup_counter debugfs entry

[Abdurrahman Hussain]

The ADM1266 maintains a 16-bit non-volatile POWERUP_COUNTER register
(0xE4, datasheet Rev. D, Table 93) that increments on every power
cycle and cannot be reset by the host. Each blackbox record already
embeds the counter at record time, so the standalone live value is
primarily useful for matching a captured record back to the boot it
came from when correlating logs.

Expose it as a read-only debugfs file alongside firmware_revision.
The block-read returns two payload bytes in little-endian order.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/hwmon/pmbus/adm1266.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index bd3784d59a7c..080e7dbd0c06 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -28,6 +28,7 @@
 #define ADM1266_READ_BLACKBOX	0xDE
 #define ADM1266_SET_RTC		0xDF
 #define ADM1266_GPIO_CONFIG	0xE1
+#define ADM1266_POWERUP_COUNTER	0xE4
 #define ADM1266_BLACKBOX_INFO	0xE6
 #define ADM1266_PDIO_STATUS	0xE9
 #define ADM1266_GPIO_STATUS	0xEA
@@ -356,6 +357,31 @@ static int adm1266_firmware_revision_read(struct seq_file *s, void *pdata)
 	return 0;
 }
 
+/*
+ * POWERUP_COUNTER (0xE4) is a 2-byte little-endian non-volatile counter
+ * that increments on every device power cycle (datasheet Rev. D, Table
+ * 93). It saturates at 65535 and cannot be reset by the host. Each
+ * blackbox record embeds the counter value at record time, so this live
+ * read is mainly useful for matching a record back to its boot.
+ */
+static int adm1266_powerup_counter_read(struct seq_file *s, void *pdata)
+{
+	struct device *dev = s->private;
+	struct i2c_client *client = to_i2c_client(dev);
+	u8 buf[I2C_SMBUS_BLOCK_MAX];
+	int ret;
+
+	ret = i2c_smbus_read_block_data(client, ADM1266_POWERUP_COUNTER, buf);
+	if (ret < 0)
+		return ret;
+	if (ret != 2)
+		return -EIO;
+
+	seq_printf(s, "%u\n", buf[0] | (buf[1] << 8));
+
+	return 0;
+}
+
 /*
  * Clearing the blackbox is required when the device is configured in
  * single-recording mode (BLACKBOX_CONFIG[0] = 0): once the 32-record
@@ -404,6 +430,8 @@ static void adm1266_init_debugfs(struct adm1266_data *data)
 				    adm1266_state_read);
 	debugfs_create_devm_seqfile(&data->client->dev, "firmware_revision", data->debugfs_dir,
 				    adm1266_firmware_revision_read);
+	debugfs_create_devm_seqfile(&data->client->dev, "powerup_counter", data->debugfs_dir,
+				    adm1266_powerup_counter_read);
 	debugfs_create_file("clear_blackbox", 0200, data->debugfs_dir, data->client,
 			    &adm1266_clear_blackbox_fops);
 }

-- 
2.53.0

[PATCH v3 4/5] hwmon: (pmbus/adm1266) replace probe-time RTC seed with rtc_class device

[Abdurrahman Hussain]

The driver currently writes the device's internal RTC at probe with
ktime_get_seconds(), which returns CLOCK_MONOTONIC seconds since boot
and is not a wall-clock value. The resulting timestamps embedded in
blackbox records are therefore meaningless across reboots, defeating
the cross-reboot record-correlation use case the field exists for.

Switching the seed to ktime_get_real_seconds() does not actually fix
this: at probe the system wall clock may not yet have been set (no
external RTC, no userspace NTP), and seeding unconditionally also
clobbers whatever valid time the ADM1266 retained across a warm
reboot.

The data sheet (Rev. D, p. 22) recommends "frequently send the time
stamp to the ADM1266 to synchronize the UNIX time and reduce the time
from drifting" when running on the internal oscillator. The clean way
to expose that policy is an rtc_class device backed by SET_RTC, so
that userspace tooling (hwclock, chrony, systemd-timesyncd) can drive
the re-sync against /dev/rtcN once it trusts the system clock - with
no driver-specific sysfs ABI.

Drop the probe-time seed and adm1266_set_rtc() entirely. Add an
rtc_class device whose ->read_time and ->set_time callbacks read and
write the SET_RTC frame. The rtc_class API is second-precision, so
the SET_RTC fractional-seconds bytes are always written as zero.

Fixes: 15609d189302 ("hwmon: (pmbus/adm1266) read blackbox")
Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/hwmon/pmbus/adm1266.c | 78 +++++++++++++++++++++++++++++++++++--------
 1 file changed, 65 insertions(+), 13 deletions(-)

diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index 080e7dbd0c06..05b31bb08f38 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -18,8 +18,8 @@
 #include <linux/nvmem-consumer.h>
 #include <linux/nvmem-provider.h>
 #include "pmbus.h"
+#include <linux/rtc.h>
 #include <linux/slab.h>
-#include <linux/timekeeping.h>
 
 #define ADM1266_IC_DEVICE_REV	0xAE
 #define ADM1266_BLACKBOX_CONFIG	0xD3
@@ -517,21 +517,73 @@ static int adm1266_config_nvmem(struct adm1266_data *data)
 	return 0;
 }
 
-static int adm1266_set_rtc(struct adm1266_data *data)
+/*
+ * SET_RTC frame layout (datasheet Rev. D, Table 84):
+ *   bytes [1:0] = fractional seconds, LSB = 1/65536 s
+ *   bytes [5:2] = seconds since 1970-01-01 UTC
+ * The rtc_class API is second-precision, so the fractional bytes are
+ * always written as zero.
+ */
+static int adm1266_write_rtc(struct i2c_client *client, time64_t secs)
 {
-	time64_t kt;
-	char write_buf[6];
+	u8 buf[6] = { 0 };
 	int i;
 
-	kt = ktime_get_seconds();
+	for (i = 0; i < 4; i++)
+		buf[2 + i] = (secs >> (i * 8)) & 0xFF;
+
+	return i2c_smbus_write_block_data(client, ADM1266_SET_RTC, sizeof(buf), buf);
+}
+
+static int adm1266_rtc_read_time(struct device *dev, struct rtc_time *tm)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+	u8 buf[I2C_SMBUS_BLOCK_MAX];
+	u32 secs;
+	int ret;
+	int i;
 
-	memset(write_buf, 0, sizeof(write_buf));
+	guard(pmbus_lock)(client);
+	ret = i2c_smbus_read_block_data(client, ADM1266_SET_RTC, buf);
+	if (ret < 0)
+		return ret;
+	if (ret != 6)
+		return -EIO;
 
+	secs = 0;
 	for (i = 0; i < 4; i++)
-		write_buf[2 + i] = (kt >> (i * 8)) & 0xFF;
+		secs |= (u32)buf[2 + i] << (i * 8);
+
+	rtc_time64_to_tm(secs, tm);
+	return 0;
+}
 
-	return i2c_smbus_write_block_data(data->client, ADM1266_SET_RTC, sizeof(write_buf),
-					  write_buf);
+static int adm1266_rtc_set_time(struct device *dev, struct rtc_time *tm)
+{
+	struct i2c_client *client = to_i2c_client(dev);
+
+	guard(pmbus_lock)(client);
+	return adm1266_write_rtc(client, rtc_tm_to_time64(tm));
+}
+
+static const struct rtc_class_ops adm1266_rtc_ops = {
+	.read_time = adm1266_rtc_read_time,
+	.set_time = adm1266_rtc_set_time,
+};
+
+static int adm1266_register_rtc(struct adm1266_data *data)
+{
+	struct rtc_device *rtc;
+
+	rtc = devm_rtc_allocate_device(&data->client->dev);
+	if (IS_ERR(rtc))
+		return PTR_ERR(rtc);
+
+	rtc->ops = &adm1266_rtc_ops;
+	rtc->range_min = 0;
+	rtc->range_max = U32_MAX;
+
+	return devm_rtc_register_device(rtc);
 }
 
 static int adm1266_probe(struct i2c_client *client)
@@ -557,10 +609,6 @@ static int adm1266_probe(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
-	ret = adm1266_set_rtc(data);
-	if (ret < 0)
-		return ret;
-
 	ret = pmbus_do_probe(client, &data->info);
 	if (ret)
 		return ret;
@@ -569,6 +617,10 @@ static int adm1266_probe(struct i2c_client *client)
 	if (ret < 0)
 		return ret;
 
+	ret = adm1266_register_rtc(data);
+	if (ret < 0)
+		return ret;
+
 	adm1266_init_debugfs(data);
 
 	return 0;

-- 
2.53.0

[PATCH v3 5/5] hwmon: (pmbus/adm1266) include adapter number in GPIO line label

[Abdurrahman Hussain]

Platforms that fit more than one ADM1266 on different I2C buses at
the same 7-bit slave address (a common shelf-management pattern,
e.g. one device per power domain) end up with duplicate GPIO line
labels because the existing format only includes the slave address.
Including the adapter number disambiguates them.

The adapter number is formatted as decimal to match the i2c-N
convention used elsewhere in Linux (sysfs paths, dev nodes); the
slave address keeps its conventional hexadecimal form.

The label is purely informational (visible via gpioinfo and the
gpiochip /sys/class/gpio name); no DT or ABI consumer parses it.

Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
---
 drivers/hwmon/pmbus/adm1266.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
index 05b31bb08f38..12cdf6de341a 100644
--- a/drivers/hwmon/pmbus/adm1266.c
+++ b/drivers/hwmon/pmbus/adm1266.c
@@ -292,8 +292,9 @@ static int adm1266_config_gpio(struct adm1266_data *data)
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(data->gpio_names); i++) {
-		gpio_name = devm_kasprintf(&data->client->dev, GFP_KERNEL, "adm1266-%x-%s",
-					   data->client->addr, adm1266_names[i]);
+		gpio_name = devm_kasprintf(&data->client->dev, GFP_KERNEL, "adm1266-%d-%x-%s",
+					   data->client->adapter->nr, data->client->addr,
+					   adm1266_names[i]);
 		if (!gpio_name)
 			return -ENOMEM;
 

-- 
2.53.0

Re: [PATCH v3 2/5] hwmon: (pmbus/adm1266) add clear_blackbox debugfs entry

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] The shared buffer `data->dev_mem` is modified and accessed in `adm1266_nvmem_read()` outside of any lock, leading to data corruption on concurrent nvmem reads.
- [Critical] Stack buffer overflow in `adm1266_nvmem_read_blackbox()` caused by an undersized buffer passed to `i2c_smbus_read_block_data()`.
- [Critical] Heap buffer overflow in `adm1266_nvmem_read_blackbox()` due to blindly trusting the hardware-provided `record_count`.
- [Critical] Out-of-bounds write in `adm1266_pmbus_block_xfer()` due to unsafe `memcpy` using an untrusted length from the I2C device.
- [High] DMA read buffer bounds mismatch in `adm1266_pmbus_block_xfer()` exposes an out-of-bounds write vulnerability.
--

commit 2afefd44ddcfcdd2fe5f8721e42653261b4e9ff0
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

hwmon: (pmbus/adm1266) add clear_blackbox debugfs entry

This commit adds a write-only debugfs file to clear the ADM1266 blackbox
buffer. Writing to the file issues a 2-byte block-write command to the
device. It also adds a pmbus_lock guard to adm1266_nvmem_read_blackbox()
to prevent concurrent read/write operations from interleaving.

> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index 7b8433226176f..bd3784d59a7c8 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c
[ ... ]
> @@ -379,10 +415,11 @@ static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)
>  	u8 buf[5];
>  	int ret;
>  
> +	guard(pmbus_lock)(data->client);
> +

While this patch adds a guard here, does adm1266_nvmem_read() also modify
and access data->dev_mem without taking this lock?

If two threads read the nvmem device concurrently, could one thread's
memset() execute during another thread's memcpy(), causing corrupted data
to be copied to userspace?

>  	ret = i2c_smbus_read_block_data(data->client, ADM1266_BLACKBOX_INFO, buf);

Can this call overflow the buf array?

Looking at i2c_smbus_read_block_data(), it blindly copies up to
I2C_SMBUS_BLOCK_MAX (32) bytes into the provided buffer based on the
length byte returned by the hardware:

drivers/i2c/i2c-core-smbus.c:i2c_smbus_read_block_data() {
    ...
	status = i2c_smbus_xfer(...);
	if (status)
		return status;

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

If the I2C device returns a block length greater than 5, would this
memcpy() overflow the 5-byte buf array?

>  	if (ret < 0)
>  		return ret;
> -
>  	if (ret != 4)
>  		return -EIO;
>  
> @@ -392,7 +429,6 @@ static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)
>  		ret = adm1266_pmbus_block_xfer(data, ADM1266_READ_BLACKBOX, 1, &index, read_buff);

Does this loop have a bounds checking issue?

The record_count loop limit is read directly from buf[3]. If the device
returns a record_count greater than 32, will the loop advance read_buff
past the end of the 2048-byte allocation for data->dev_mem?

Also, could this call to adm1266_pmbus_block_xfer() cause an
out-of-bounds write?

In adm1266_pmbus_block_xfer(), the memcpy() uses a length from the I2C
device:

drivers/hwmon/pmbus/adm1266.c:adm1266_pmbus_block_xfer() {
    ...
	memcpy(data_r, &msgs[1].buf[1], msgs[1].buf[0]);
    ...
}

If the device returns a length up to 255, could this write past the
expected 64 bytes (ADM1266_BLACKBOX_SIZE) allocated for read_buff?

Additionally, does the DMA read buffer size in adm1266_pmbus_block_xfer()
expose an overflow regression?

The read_buf array in struct adm1266_data is sized
ADM1266_PMBUS_BLOCK_MAX + 1 (256 bytes). However, the i2c_msg length is
set to 257:

drivers/hwmon/pmbus/adm1266.c:adm1266_pmbus_block_xfer() {
    ...
		{
			.addr = client->addr,
			.flags = I2C_M_RD | I2C_M_DMA_SAFE,
			.buf = data->read_buf,
			.len = ADM1266_PMBUS_BLOCK_MAX + 2,
		}
    ...
}

If the hardware sends a full 257-byte response, will the controller
write 1 byte past the end of data->read_buf?

>  		if (ret < 0)
>  			return ret;
> -
>  		if (ret != ADM1266_BLACKBOX_SIZE)
>  			return -EIO;
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512-adm1266-v3-0-a81a479b0bb0@nexthop.ai?part=2

Re: [PATCH v3 3/5] hwmon: (pmbus/adm1266) add powerup_counter debugfs entry

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Missing locking in debugfs read callback `adm1266_powerup_counter_read` violates subsystem concurrency guidelines and allows interleaving with PMBus sequences.
--

commit d2c76f8ee7556a97e62b160e80fa96f155d632ed
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

hwmon: (pmbus/adm1266) add powerup_counter debugfs entry

This commit exposes the adm1266 16-bit non-volatile powerup counter as a
read-only debugfs file. The live value is read using a block-read to
retrieve two payload bytes in little-endian order.

> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index bd3784d59a7c8..080e7dbd0c069 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c
[ ... ]
> @@ -356,6 +357,31 @@ static int adm1266_firmware_revision_read(struct seq_file *s, void *pdata)
>  	return 0;
>  }
>  
> +/*
> + * POWERUP_COUNTER (0xE4) is a 2-byte little-endian non-volatile counter
> + * that increments on every device power cycle (datasheet Rev. D, Table
> + * 93). It saturates at 65535 and cannot be reset by the host. Each
> + * blackbox record embeds the counter value at record time, so this live
> + * read is mainly useful for matching a record back to its boot.
> + */
> +static int adm1266_powerup_counter_read(struct seq_file *s, void *pdata)
> +{
> +	struct device *dev = s->private;
> +	struct i2c_client *client = to_i2c_client(dev);
> +	u8 buf[I2C_SMBUS_BLOCK_MAX];
> +	int ret;
> +
> +	ret = i2c_smbus_read_block_data(client, ADM1266_POWERUP_COUNTER, buf);

Does this lockless hardware access introduce a regression by violating hwmon
subsystem concurrency guidelines?

The hwmon guidelines specify that drivers must implement locking for
attributes registered by any other means, such as debugfs files. While a
single i2c transaction is serialized at the adapter level, a lockless
access here could interleave with stateful multi-transaction pmbus
sequences on other threads, such as setting a page register followed by
a read.

Should this function acquire pmbus_lock(client) to safely serialize the
hardware access and prevent disruption of the device internal state machine?

> +	if (ret < 0)
> +		return ret;
> +	if (ret != 2)
> +		return -EIO;
> +
> +	seq_printf(s, "%u\n", buf[0] | (buf[1] << 8));
> +
> +	return 0;
> +}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512-adm1266-v3-0-a81a479b0bb0@nexthop.ai?part=3

Re: [PATCH v3 4/5] hwmon: (pmbus/adm1266) replace probe-time RTC seed with rtc_class device

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Missing Kconfig dependency on RTC_CLASS causing potential build failures.
--

commit f54ef3c53db957606d5afd59cb615359c244ece4
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>

hwmon: (pmbus/adm1266) replace probe-time RTC seed with rtc_class device

This commit drops the probe-time seed and adm1266_set_rtc() entirely. It adds
an rtc_class device whose read_time and set_time callbacks read and write the
SET_RTC frame to expose the time stamp policy properly.

> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index 080e7dbd0c069..05b31bb08f38c 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c

[ ... ]

> @@ -517,21 +517,73 @@ static int adm1266_config_nvmem(struct adm1266_data *data)

[ ... ]

> +static int adm1266_register_rtc(struct adm1266_data *data)
> +{
> +	struct rtc_device *rtc;
> +
> +	rtc = devm_rtc_allocate_device(&data->client->dev);

Could this introduce a build regression if CONFIG_RTC_CLASS is disabled?

The patch unconditionally adds calls to RTC subsystem functions like
devm_rtc_allocate_device() and devm_rtc_register_device(). These functions
are implemented in the RTC core, which is governed by CONFIG_RTC_CLASS.

There are no fallback stubs in linux/rtc.h for these allocation functions
when the subsystem is disabled. Does CONFIG_SENSORS_ADM1266 need a dependency
on RTC_CLASS in drivers/hwmon/pmbus/Kconfig to prevent unresolved external
symbols during linking?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512-adm1266-v3-0-a81a479b0bb0@nexthop.ai?part=4

Re: [PATCH v3 4/5] hwmon: (pmbus/adm1266) replace probe-time RTC seed with rtc_class device

[Guenter Roeck]

On 5/12/26 11:56, Abdurrahman Hussain wrote:
> The driver currently writes the device's internal RTC at probe with
> ktime_get_seconds(), which returns CLOCK_MONOTONIC seconds since boot
> and is not a wall-clock value. The resulting timestamps embedded in
> blackbox records are therefore meaningless across reboots, defeating
> the cross-reboot record-correlation use case the field exists for.
> 
> Switching the seed to ktime_get_real_seconds() does not actually fix
> this: at probe the system wall clock may not yet have been set (no
> external RTC, no userspace NTP), and seeding unconditionally also
> clobbers whatever valid time the ADM1266 retained across a warm
> reboot.
> 
> The data sheet (Rev. D, p. 22) recommends "frequently send the time
> stamp to the ADM1266 to synchronize the UNIX time and reduce the time
> from drifting" when running on the internal oscillator. The clean way
> to expose that policy is an rtc_class device backed by SET_RTC, so
> that userspace tooling (hwclock, chrony, systemd-timesyncd) can drive
> the re-sync against /dev/rtcN once it trusts the system clock - with
> no driver-specific sysfs ABI.
> 
> Drop the probe-time seed and adm1266_set_rtc() entirely. Add an
> rtc_class device whose ->read_time and ->set_time callbacks read and
> write the SET_RTC frame. The rtc_class API is second-precision, so
> the SET_RTC fractional-seconds bytes are always written as zero.
> 

This doesn't just set the RTC time, it also acts as a real-time-clock.
That seems undesirable, since it isn't really a real-time clock.
It would also be the first (pseudo) rtc residing outside the rtc
subsystem.

It seems to me that there must be a better solution. The rtc subsystem
needs to be involved in this discussion.

Note - as sent separately - that making this driver dependent on
the RTC subsystem (as would be required by this patch) is unacceptable.

Guenter

> Fixes: 15609d189302 ("hwmon: (pmbus/adm1266) read blackbox")
> Signed-off-by: Abdurrahman Hussain <abdurrahman@nexthop.ai>
> ---
>   drivers/hwmon/pmbus/adm1266.c | 78 +++++++++++++++++++++++++++++++++++--------
>   1 file changed, 65 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index 080e7dbd0c06..05b31bb08f38 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c
> @@ -18,8 +18,8 @@
>   #include <linux/nvmem-consumer.h>
>   #include <linux/nvmem-provider.h>
>   #include "pmbus.h"
> +#include <linux/rtc.h>
>   #include <linux/slab.h>
> -#include <linux/timekeeping.h>
>   
>   #define ADM1266_IC_DEVICE_REV	0xAE
>   #define ADM1266_BLACKBOX_CONFIG	0xD3
> @@ -517,21 +517,73 @@ static int adm1266_config_nvmem(struct adm1266_data *data)
>   	return 0;
>   }
>   
> -static int adm1266_set_rtc(struct adm1266_data *data)
> +/*
> + * SET_RTC frame layout (datasheet Rev. D, Table 84):
> + *   bytes [1:0] = fractional seconds, LSB = 1/65536 s
> + *   bytes [5:2] = seconds since 1970-01-01 UTC
> + * The rtc_class API is second-precision, so the fractional bytes are
> + * always written as zero.
> + */
> +static int adm1266_write_rtc(struct i2c_client *client, time64_t secs)
>   {
> -	time64_t kt;
> -	char write_buf[6];
> +	u8 buf[6] = { 0 };
>   	int i;
>   
> -	kt = ktime_get_seconds();
> +	for (i = 0; i < 4; i++)
> +		buf[2 + i] = (secs >> (i * 8)) & 0xFF;
> +
> +	return i2c_smbus_write_block_data(client, ADM1266_SET_RTC, sizeof(buf), buf);
> +}
> +
> +static int adm1266_rtc_read_time(struct device *dev, struct rtc_time *tm)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +	u8 buf[I2C_SMBUS_BLOCK_MAX];
> +	u32 secs;
> +	int ret;
> +	int i;
>   
> -	memset(write_buf, 0, sizeof(write_buf));
> +	guard(pmbus_lock)(client);
> +	ret = i2c_smbus_read_block_data(client, ADM1266_SET_RTC, buf);
> +	if (ret < 0)
> +		return ret;
> +	if (ret != 6)
> +		return -EIO;
>   
> +	secs = 0;
>   	for (i = 0; i < 4; i++)
> -		write_buf[2 + i] = (kt >> (i * 8)) & 0xFF;
> +		secs |= (u32)buf[2 + i] << (i * 8);
> +
> +	rtc_time64_to_tm(secs, tm);
> +	return 0;
> +}
>   
> -	return i2c_smbus_write_block_data(data->client, ADM1266_SET_RTC, sizeof(write_buf),
> -					  write_buf);
> +static int adm1266_rtc_set_time(struct device *dev, struct rtc_time *tm)
> +{
> +	struct i2c_client *client = to_i2c_client(dev);
> +
> +	guard(pmbus_lock)(client);
> +	return adm1266_write_rtc(client, rtc_tm_to_time64(tm));
> +}
> +
> +static const struct rtc_class_ops adm1266_rtc_ops = {
> +	.read_time = adm1266_rtc_read_time,
> +	.set_time = adm1266_rtc_set_time,
> +};
> +
> +static int adm1266_register_rtc(struct adm1266_data *data)
> +{
> +	struct rtc_device *rtc;
> +
> +	rtc = devm_rtc_allocate_device(&data->client->dev);
> +	if (IS_ERR(rtc))
> +		return PTR_ERR(rtc);
> +
> +	rtc->ops = &adm1266_rtc_ops;
> +	rtc->range_min = 0;
> +	rtc->range_max = U32_MAX;
> +
> +	return devm_rtc_register_device(rtc);
>   }
>   
>   static int adm1266_probe(struct i2c_client *client)
> @@ -557,10 +609,6 @@ static int adm1266_probe(struct i2c_client *client)
>   	if (ret < 0)
>   		return ret;
>   
> -	ret = adm1266_set_rtc(data);
> -	if (ret < 0)
> -		return ret;
> -
>   	ret = pmbus_do_probe(client, &data->info);
>   	if (ret)
>   		return ret;
> @@ -569,6 +617,10 @@ static int adm1266_probe(struct i2c_client *client)
>   	if (ret < 0)
>   		return ret;
>   
> +	ret = adm1266_register_rtc(data);
> +	if (ret < 0)
> +		return ret;
> +
>   	adm1266_init_debugfs(data);
>   
>   	return 0;
> 

Re: [PATCH v3 4/5] hwmon: (pmbus/adm1266) replace probe-time RTC seed with rtc_class device

[Guenter Roeck]

On 5/13/26 18:57, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Low] Missing Kconfig dependency on RTC_CLASS causing potential build failures.
> --
> 
> commit f54ef3c53db957606d5afd59cb615359c244ece4
> Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>
> 
> hwmon: (pmbus/adm1266) replace probe-time RTC seed with rtc_class device
> 
> This commit drops the probe-time seed and adm1266_set_rtc() entirely. It adds
> an rtc_class device whose read_time and set_time callbacks read and write the
> SET_RTC frame to expose the time stamp policy properly.
> 
>> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
>> index 080e7dbd0c069..05b31bb08f38c 100644
>> --- a/drivers/hwmon/pmbus/adm1266.c
>> +++ b/drivers/hwmon/pmbus/adm1266.c
> 
> [ ... ]
> 
>> @@ -517,21 +517,73 @@ static int adm1266_config_nvmem(struct adm1266_data *data)
> 
> [ ... ]
> 
>> +static int adm1266_register_rtc(struct adm1266_data *data)
>> +{
>> +	struct rtc_device *rtc;
>> +
>> +	rtc = devm_rtc_allocate_device(&data->client->dev);
> 
> Could this introduce a build regression if CONFIG_RTC_CLASS is disabled?
> 
> The patch unconditionally adds calls to RTC subsystem functions like
> devm_rtc_allocate_device() and devm_rtc_register_device(). These functions
> are implemented in the RTC core, which is governed by CONFIG_RTC_CLASS.
> 
> There are no fallback stubs in linux/rtc.h for these allocation functions
> when the subsystem is disabled. Does CONFIG_SENSORS_ADM1266 need a dependency
> on RTC_CLASS in drivers/hwmon/pmbus/Kconfig to prevent unresolved external
> symbols during linking?
> 

That would be unacceptable. The driver has to work on systems with no RTC.

Guenter

Re: [PATCH v3 4/5] hwmon: (pmbus/adm1266) replace probe-time RTC seed with rtc_class device

[Abdurrahman Hussain]

On Wed May 13, 2026 at 7:14 PM PDT, Guenter Roeck wrote:
> On 5/12/26 11:56, Abdurrahman Hussain wrote:
>> The driver currently writes the device's internal RTC at probe with
>> ktime_get_seconds(), which returns CLOCK_MONOTONIC seconds since boot
>> and is not a wall-clock value. The resulting timestamps embedded in
>> blackbox records are therefore meaningless across reboots, defeating
>> the cross-reboot record-correlation use case the field exists for.
>> 
>> Switching the seed to ktime_get_real_seconds() does not actually fix
>> this: at probe the system wall clock may not yet have been set (no
>> external RTC, no userspace NTP), and seeding unconditionally also
>> clobbers whatever valid time the ADM1266 retained across a warm
>> reboot.
>> 
>> The data sheet (Rev. D, p. 22) recommends "frequently send the time
>> stamp to the ADM1266 to synchronize the UNIX time and reduce the time
>> from drifting" when running on the internal oscillator. The clean way
>> to expose that policy is an rtc_class device backed by SET_RTC, so
>> that userspace tooling (hwclock, chrony, systemd-timesyncd) can drive
>> the re-sync against /dev/rtcN once it trusts the system clock - with
>> no driver-specific sysfs ABI.
>> 
>> Drop the probe-time seed and adm1266_set_rtc() entirely. Add an
>> rtc_class device whose ->read_time and ->set_time callbacks read and
>> write the SET_RTC frame. The rtc_class API is second-precision, so
>> the SET_RTC fractional-seconds bytes are always written as zero.
>> 
>
> This doesn't just set the RTC time, it also acts as a real-time-clock.
> That seems undesirable, since it isn't really a real-time clock.
> It would also be the first (pseudo) rtc residing outside the rtc
> subsystem.
>

Agreed -- calling it an RTC overstates what the chip does.  The
register is a 32-bit seconds counter seeded via SET_RTC and stamped
into each blackbox record.  No battery, no alarm, nothing the RTC API
normally implies; treating it as an rtc_class device was the wrong
fit.

> It seems to me that there must be a better solution. The rtc subsystem
> needs to be involved in this discussion.
>

Would you prefer I drop anything RTC-shaped from this driver
altogether and reach for something else (e.g. a read/write debugfs
file under pmbus/<dev>/, consistent with the other debugfs entries
this series adds), or would you rather I post a separate thread to
linux-rtc asking how a device like this should expose a host-driven
seconds counter and wait on their guidance before respinning?

The datasheet's intended use is for the host to frequently send the
current timestamp to the chip, so the value stamped into each blackbox
record reflects wall-clock time at the moment of the event.  Either
direction works for me; my mild preference is the debugfs path since
it lets me close the loop without holding up the rest of the series.

> Note - as sent separately - that making this driver dependent on
> the RTC subsystem (as would be required by this patch) is unacceptable.
>

Understood -- I'll drop the rtc_class approach in v4 either way.

Thanks for the quick feedback,
Abdurrahman

Re: [PATCH v3 2/5] hwmon: (pmbus/adm1266) add clear_blackbox debugfs entry

[Abdurrahman Hussain]

On Wed May 13, 2026 at 5:47 PM PDT, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [Critical] The shared buffer `data->dev_mem` is modified and accessed in `adm1266_nvmem_read()` outside of any lock, leading to data corruption on concurrent nvmem reads.
> - [Critical] Stack buffer overflow in `adm1266_nvmem_read_blackbox()` caused by an undersized buffer passed to `i2c_smbus_read_block_data()`.
> - [Critical] Heap buffer overflow in `adm1266_nvmem_read_blackbox()` due to blindly trusting the hardware-provided `record_count`.
> - [Critical] Out-of-bounds write in `adm1266_pmbus_block_xfer()` due to unsafe `memcpy` using an untrusted length from the I2C device.
> - [High] DMA read buffer bounds mismatch in `adm1266_pmbus_block_xfer()` exposes an out-of-bounds write vulnerability.
> --
>
> commit 2afefd44ddcfcdd2fe5f8721e42653261b4e9ff0
> Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>
>
> hwmon: (pmbus/adm1266) add clear_blackbox debugfs entry
>
> This commit adds a write-only debugfs file to clear the ADM1266 blackbox
> buffer. Writing to the file issues a 2-byte block-write command to the
> device. It also adds a pmbus_lock guard to adm1266_nvmem_read_blackbox()
> to prevent concurrent read/write operations from interleaving.
>
>> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
>> index 7b8433226176f..bd3784d59a7c8 100644
>> --- a/drivers/hwmon/pmbus/adm1266.c
>> +++ b/drivers/hwmon/pmbus/adm1266.c
> [ ... ]
>> @@ -379,10 +415,11 @@ static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)
>>  	u8 buf[5];
>>  	int ret;
>>  
>> +	guard(pmbus_lock)(data->client);
>> +
>
> While this patch adds a guard here, does adm1266_nvmem_read() also modify
> and access data->dev_mem without taking this lock?
>
> If two threads read the nvmem device concurrently, could one thread's
> memset() execute during another thread's memcpy(), causing corrupted data
> to be copied to userspace?
>

Yes, this is a real race -- the nvmem core (nvmem_reg_read() in
drivers/nvmem/core.c) does not serialize reg_read callbacks, so two
concurrent reads of the nvmem device can interleave the memset and
memcpy on data->dev_mem.  For v4 I'll lift the guard from
adm1266_nvmem_read_blackbox() up into adm1266_nvmem_read() so it
covers the memset, the blackbox refill, and the memcpy as a single
critical section.

>>  	ret = i2c_smbus_read_block_data(data->client, ADM1266_BLACKBOX_INFO, buf);
>
> Can this call overflow the buf array?
>
> Looking at i2c_smbus_read_block_data(), it blindly copies up to
> I2C_SMBUS_BLOCK_MAX (32) bytes into the provided buffer based on the
> length byte returned by the hardware:
>
> drivers/i2c/i2c-core-smbus.c:i2c_smbus_read_block_data() {
>     ...
> 	status = i2c_smbus_xfer(...);
> 	if (status)
> 		return status;
>
> 	memcpy(values, &data.block[1], data.block[0]);
> 	return data.block[0];
> }
>
> If the I2C device returns a block length greater than 5, would this
> memcpy() overflow the 5-byte buf array?
>

Yes -- a malformed length byte from the device would overflow the
5-byte buf onto the stack.  This is pre-existing in the original
driver; I'll send a separate fix patch with a Fixes: tag sizing buf
at I2C_SMBUS_BLOCK_MAX, and rebase the feature series on top.

>>  	if (ret < 0)
>>  		return ret;
>> -
>>  	if (ret != 4)
>>  		return -EIO;
>>  
>> @@ -392,7 +429,6 @@ static int adm1266_nvmem_read_blackbox(struct adm1266_data *data, u8 *read_buff)
>>  		ret = adm1266_pmbus_block_xfer(data, ADM1266_READ_BLACKBOX, 1, &index, read_buff);
>
> Does this loop have a bounds checking issue?
>
> The record_count loop limit is read directly from buf[3]. If the device
> returns a record_count greater than 32, will the loop advance read_buff
> past the end of the 2048-byte allocation for data->dev_mem?
>

Yes; record_count comes straight from the device.  Pre-existing as
well -- same fix path.  v4 will clamp against the declared blackbox
size (32 records) before entering the loop and return -EIO if the
device reports more.

> Also, could this call to adm1266_pmbus_block_xfer() cause an
> out-of-bounds write?
>
> In adm1266_pmbus_block_xfer(), the memcpy() uses a length from the I2C
> device:
>
> drivers/hwmon/pmbus/adm1266.c:adm1266_pmbus_block_xfer() {
>     ...
> 	memcpy(data_r, &msgs[1].buf[1], msgs[1].buf[0]);
>     ...
> }
>
> If the device returns a length up to 255, could this write past the
> expected 64 bytes (ADM1266_BLACKBOX_SIZE) allocated for read_buff?
>

Yes.  adm1266_pmbus_block_xfer() memcpy()s up to 255 bytes into a
caller-supplied buffer whose size the function has no knowledge of.
Pre-existing; same fix path.  The fix will add a max-length
parameter to the helper and update the three call sites to pass
their actual buffer size.

> Additionally, does the DMA read buffer size in adm1266_pmbus_block_xfer()
> expose an overflow regression?
>
> The read_buf array in struct adm1266_data is sized
> ADM1266_PMBUS_BLOCK_MAX + 1 (256 bytes). However, the i2c_msg length is
> set to 257:
>
> drivers/hwmon/pmbus/adm1266.c:adm1266_pmbus_block_xfer() {
>     ...
> 		{
> 			.addr = client->addr,
> 			.flags = I2C_M_RD | I2C_M_DMA_SAFE,
> 			.buf = data->read_buf,
> 			.len = ADM1266_PMBUS_BLOCK_MAX + 2,
> 		}
>     ...
> }
>
> If the hardware sends a full 257-byte response, will the controller
> write 1 byte past the end of data->read_buf?
>
>>  		if (ret < 0)
>>  			return ret;
>> -
>>  		if (ret != ADM1266_BLACKBOX_SIZE)
>>  			return -EIO;
>>

Yes -- read_buf needs ADM1266_PMBUS_BLOCK_MAX + 2 (257 bytes) to hold
the length byte plus up to 255 payload bytes plus the PEC byte.  The
subsequent PEC compare,

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

also reads past the end of the current array for a max-length
response.  Pre-existing; same fix path.

In summary, the four pre-existing buffer-bound bugs (points 2-5
above) will go into a separate fix series with Fixes: tags sent
first; the nvmem-read race (point 1) is folded into this patch in
v4.

Thanks,
Abdurrahman