Re: [PATCH] Documentation/hwmon: Document hwmon_notify_event()

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

Pre-existing issues:
- [High] The pec_store() function violates subsystem cleanup guidelines by mixing guard() lock acquisition with goto-based error handling, resulting in a Use-After-Free vulnerability.
--

--- Patch [1]: [PATCH] Documentation/hwmon: Document hwmon_notify_event() ---
commit 2b10b27f5fc8a96731a2e5192ecb3af70d5ee8cc
Author: Guenter Roeck <[email protected]>

Documentation/hwmon: Document hwmon_notify_event()

Add the function prototype and description of hwmon_notify_event() to
the hwmon kernel API guide. This allows drivers to report events such as
alarms or faults to userspace.

> diff --git a/Documentation/hwmon/hwmon-kernel-api.rst b/Documentation/hwmon/hwmon-kernel-api.rst
> index 9fcde32a140df..c3eb433a78f61 100644
> --- a/Documentation/hwmon/hwmon-kernel-api.rst
> +++ b/Documentation/hwmon/hwmon-kernel-api.rst
> @@ -42,6 +42,9 @@ register/unregister functions::
>  
>    char *devm_hwmon_sanitize_name(struct device *dev, const char *name);
>  
> +  int hwmon_notify_event(struct device *dev, enum hwmon_sensor_types type,
> +			 u32 attr, int channel);
> +
>    void hwmon_lock(struct device *dev);
>    void hwmon_unlock(struct device *dev);
>  

[ ... ]

This is a pre-existing issue, but while reviewing the hwmon subsystem I noticed
a potential bug in drivers/hwmon/hwmon.c:

[Severity: High]
Can this sequence trigger a use-after-free?

Looking at pec_store() in drivers/hwmon/hwmon.c, we acquire a device reference
and lock, but mix it with a goto statement for error handling:

drivers/hwmon/hwmon.c:pec_store() {
        ...
        hdev = device_find_child(dev, NULL, hwmon_match_device);
        ...
        hwdev = to_hwmon_device(hdev);
        guard(mutex)(&hwdev->lock);
        if (hwdev->chip->ops->write) {
                err = hwdev->chip->ops->write(hdev, hwmon_chip, hwmon_chip_pec, 0, val);
                if (err && err != -EOPNOTSUPP)
                        goto put;
        }
        ...
put:
        put_device(hdev);

        return err;
}

If the put_device(hdev) call drops the last reference to the device (for
example, due to a concurrent device removal), it triggers hwmon_dev_release()
which frees the hwdev memory.

When the function returns, the guard scope exits and attempts to call
mutex_unlock() on the saved hwdev->lock pointer, dereferencing the freshly
freed memory.

Could this be converted to use scoped_guard() or avoid the goto statement to
ensure the lock is released before dropping the device reference?

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