Re: [PATCH v3 4/8] hwmon: (pmbus/adm1266) register the gpio_chip after pmbus_do_probe()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] Massive heap buffer overflow in adm1266_nvmem_read_blackbox() via untrusted record_count.
- [High] Deterministic 1-byte heap out-of-bounds write in adm1266_pmbus_block_xfer() due to mismatched buffer sizes.
--

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

hwmon: (pmbus/adm1266) register the gpio_chip after pmbus_do_probe()

This commit changes the order of operations in adm1266_probe() to register
the gpio_chip only after pmbus_do_probe() has fully initialized the PMBus
state. This prevents a race condition where userspace could access the
uninitialized PMBus state via gpiolib.

> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index 57cb7d302cddd..b91dcf067fa6d 100644
> --- a/drivers/hwmon/pmbus/adm1266.c
> +++ b/drivers/hwmon/pmbus/adm1266.c

[ ... ]

This problem wasn't introduced by this patch, but can a malfunctioning or
malicious I2C device returning a large record_count cause a heap buffer
overflow here?

drivers/hwmon/pmbus/adm1266.c:adm1266_nvmem_read_blackbox() {
...
	record_count = buf[3];

	for (index = 0; index < record_count; index++) {
		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;

		read_buff += ADM1266_BLACKBOX_SIZE;
	}
...
}

The record_count is read directly from the I2C device into buf[3] (an 8-bit
unsigned value, up to 255). The function iterates record_count times without
an upper bound validation, advancing and writing 64 bytes to read_buff on each
iteration.

Since read_buff points to data->dev_mem which is exactly 2048 bytes,
could this write up to 16,320 bytes and overflow the buffer?


This problem wasn't introduced by this patch, but is it possible for this I2C
transfer to write past the end of data->read_buf?

drivers/hwmon/pmbus/adm1266.c:adm1266_pmbus_block_xfer() {
...
	struct i2c_msg msgs[2] = {
		{
...
		},
		{
			.addr = client->addr,
			.flags = I2C_M_RD | I2C_M_DMA_SAFE,
			.buf = data->read_buf,
			.len = ADM1266_PMBUS_BLOCK_MAX + 2,
		}
	};
...
	ret = i2c_transfer(client->adapter, msgs, 2);
...
}

data->read_buf is defined with size ADM1266_PMBUS_BLOCK_MAX + 1 (256 bytes).
However, msgs[1].len is hardcoded to ADM1266_PMBUS_BLOCK_MAX + 2 (257 bytes).

Because this is a raw I2C_M_RD transfer, the I2C adapter will blindly write
exactly 257 bytes into data->read_buf.

Since read_buf is the last member of the devm_kzalloc-allocated adm1266_data
struct, could this unconditionally corrupt adjacent heap memory on every
block read transfer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260518-adm1266-gpio-fixes-v3-0-e425e4f88139@nexthop.ai?part=4