Re: [PATCH 1/5] hwmon: (pmbus/core) Introduce pmbus_read_smbus_i2c_block_data()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The hwmon device is exposed to userspace before the driver probe is fully complete, causing a race condition on I2C page state.
--

--- Patch [1]: [PATCH 1/5] hwmon: (pmbus/core) Introduce pmbus_read_smbus_i2c_block_data() ---
commit 80dbae6dc3a3ada2ae6512549c713faf3d025f76
Author: Guenter Roeck <linux@roeck-us.net>

hwmon: (pmbus/core) Introduce pmbus_read_smbus_i2c_block_data()

PMBus drivers need support for reading SMBus block data. Unfortunately,
not all i2C controllers support this command.

Implement pmbus_read_smbus_i2c_block_data() which first tries to use
i2c_smbus_read_block_data(). If not supported, try to emulate it by calling
i2c_smbus_read_i2c_block_data(). Export the new function for use in PMBus
drivers.

> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index ba3e9e9fc8bcc..df2409c89a840 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c

[ ... ]

[Severity: High]
This is a pre-existing issue, but I noticed a potential race condition
during probe in pmbus_do_probe().

When devm_hwmon_device_register_with_groups() is called, the hwmon device
is exposed to userspace immediately:

drivers/hwmon/pmbus/pmbus_core.c:pmbus_do_probe() {
    ...
	data->hwmon_dev = devm_hwmon_device_register_with_groups(dev, name,
								 data, data->groups);
	if (IS_ERR(data->hwmon_dev)) {
		dev_err(dev, "Failed to register hwmon device\n");
		return PTR_ERR(data->hwmon_dev);
	}

	ret = pmbus_regulator_register(data);
	if (ret)
		return ret;

	ret = pmbus_irq_setup(client, data);
	if (ret)
		return ret;

	pmbus_init_debugfs(client, data);
    ...
}

Userspace (like udev) can immediately issue sysfs reads or writes, which hold
pmbus_lock and call pmbus_set_page().

Concurrently, the probe function continues execution and calls
pmbus_irq_setup() and pmbus_init_debugfs().

Both of these functions execute unlocked I2C operations that also call
pmbus_set_page(). Because pmbus_set_page() does not internally hold a lock,
the hardware I2C page state can become desynchronized from the software
data->currpage state.

Could this result in I2C operations being sent to the wrong page?

Since PMBus devices are often power supplies, if a sysfs write is directed to
the wrong page due to a race, could it change the voltage or limits on the
wrong power rail and potentially cause hardware damage?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803193535.2286578-1-linux@roeck-us.net?part=1