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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unsynchronized concurrent access to the shared `data->dev_mem` buffer in NVMEM read callback violates subsystem locking guidelines.
--

commit ad17d73bcc6d34ca2a117a7fde443e8a2ce1dda7
Author: Abdurrahman Hussain <abdurrahman@nexthop.ai>
Subject: hwmon: (pmbus/adm1266) widen blackbox-info buffer to I2C_SMBUS_BLOCK_MAX

adm1266_nvmem_read_blackbox() declares a 5-byte stack buffer and passes it
to i2c_smbus_read_block_data(). This patch widens the local buffer to
I2C_SMBUS_BLOCK_MAX so the helper has room for any well-formed SMBus block
response, avoiding stack buffer overflow. Note that other buffer overflow
issues are fixed in subsequent commits in this series.

> diff --git a/drivers/hwmon/pmbus/adm1266.c b/drivers/hwmon/pmbus/adm1266.c
> index a86666c73a5e9..94691dec13599 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)

This isn't a regression introduced by this patch, but does the nvmem read
callback that populates read_buff need locking?

The hardware monitoring subsystem guidelines state that drivers must implement
locking for attributes registered by other means (like nvmem) and should use
hwmon_lock() and hwmon_unlock().

Since the nvmem core does not serialize reg_read operations, can concurrent
reads from userspace via sysfs cause data races?

For example, in adm1266_nvmem_read():

	if (offset == 0) {
		memset(data->dev_mem, 0, data->nvmem_config.size);

		ret = adm1266_nvmem_read_blackbox(data, data->dev_mem);
	...
	}
	memcpy(val, data->dev_mem + offset, bytes);

If one thread executes memset() or multi-block I2C reads via
adm1266_nvmem_read_blackbox() while another thread copies partially written
data via memcpy(), could this result in memory corruption of the shared buffer
and incorrectly interlaced I2C hardware interactions?

>  {
>  	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);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260515-adm1266-fixes-v1-0-1c1ea1349cfe@nexthop.ai?part=2