Re: [RFT PATCH] hwmon: (pmbus) Fix type confusion in notification logic

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

Pre-existing issues:
- [High] sysfs_notify and kobject_uevent are called on the wrong kobject (data->dev->kobj instead of data->hwmon_dev->kobj), silently breaking hwmon event notifications.
--

--- Patch [1]: [RFT PATCH] hwmon: (pmbus) Fix type confusion in notification logic ---
commit 05911e6740d7877a722e1540f907a611d74a3925
Author: Guenter Roeck <[email protected]>

hwmon: (pmbus) Fix type confusion in notification logic

This commit fixes a potential slab-out-of-bounds read and garbage mask issue
in pmbus_notify() caused by casting non-sensor attributes to struct
sensor_device_attribute. It introduces a proper sensor_device_attribute
embedded in pmbus_sensor and pmbus_label and initializes their index to -1 so
they can be filtered out during notifications.

> diff --git a/drivers/hwmon/pmbus/pmbus_core.c b/drivers/hwmon/pmbus/pmbus_core.c
> index 3143b9e0316c4..dfe7e071017d2 100644
> --- a/drivers/hwmon/pmbus/pmbus_core.c
> +++ b/drivers/hwmon/pmbus/pmbus_core.c

[ ... ]

> @@ -2979,9 +2986,15 @@ static void pmbus_notify(struct pmbus_data *data, int page, int reg, int flags)
>  		struct device_attribute *da = to_dev_attr(data->group.attrs[i]);
>  		struct sensor_device_attribute *attr = to_sensor_dev_attr(da);
>  		int index = attr->index;
> -		u16 smask = pb_index_to_mask(index);
> -		u8 spage = pb_index_to_page(index);
> -		u16 sreg = pb_index_to_reg(index);
> +		u16 smask, sreg;
> +		u8 spage;
> +
> +		if (index == -1)
> +			continue;
> +
> +		smask = pb_index_to_mask(index);
> +		spage = pb_index_to_page(index);
> +		sreg = pb_index_to_reg(index);
>  
>  		if (reg == sreg && page == spage && (smask & flags)) {
>  			dev_dbg(data->dev, "sysfs notify: %s", da->attr.name);

[Severity: High]
This isn't a bug introduced by this patch, but is it intended to notify on
the parent I2C device kobject instead of the hwmon class device?

Looking at the subsequent lines in pmbus_notify() in
drivers/hwmon/pmbus/pmbus_core.c, sysfs_notify() and kobject_uevent() are
called using data->dev->kobj instead of data->hwmon_dev->kobj:

		if (reg == sreg && page == spage && (smask & flags)) {
			dev_dbg(data->dev, "sysfs notify: %s", da->attr.name);
			sysfs_notify(&data->dev->kobj, NULL, da->attr.name);
			kobject_uevent(&data->dev->kobj, KOBJ_CHANGE);
			flags &= ~smask;
		}

Since the hwmon attributes are registered under the hwmon class device, will
this silently fail to wake up userspace applications monitoring these sysfs
files because the target attributes are not on data->dev->kobj?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1