Re: [PATCH] hwmon: Fix potential UAF in pec_store

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

Pre-existing issues:
- [Medium] The function `pec_store()` mixes `goto`-based error handling (`goto put;`) with scope-based cleanup (`scoped_guard`), which violates subsystem guidelines.
--

--- Patch [1]: [PATCH] hwmon: Fix potential UAF in pec_store ---
commit faaeb6b5ca43335968fb5512ec894ab84e5edc9c
Author: Guenter Roeck <[email protected]>

hwmon: Fix potential UAF in pec_store

This patch fixes a potential use-after-free in pec_store() by replacing
guard() with scoped_guard(). The previous guard() usage attempted to unlock
the mutex after dropping the final device reference when jumping to the 'put'
label.

> diff --git a/drivers/hwmon/hwmon.c b/drivers/hwmon/hwmon.c
> index 29dc90a2c3fe7..8cf717ae8b1dd 100644
> --- a/drivers/hwmon/hwmon.c
> +++ b/drivers/hwmon/hwmon.c
> @@ -371,18 +371,17 @@ static ssize_t pec_store(struct device *dev, struct device_attribute *devattr,
>  	 * handling is not required.
>  	 */
>  	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;
> +	scoped_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;

[Severity: Medium]
This is a pre-existing issue, but does this code mix goto-based error
handling with scope-based cleanup helpers?

According to the subsystem guidelines in include/linux/cleanup.h, mixing goto
and cleanup helpers in the same function creates confusing ownership semantics
and should be avoided. Would it be better to also convert the 'hdev' reference
management to use __free(put_device) to fully eliminate the goto pattern?

> +		}
> +		if (!val)
> +			client->flags &= ~I2C_CLIENT_PEC;
> +		else
> +			client->flags |= I2C_CLIENT_PEC;
>  	}
> -
> -	if (!val)
> -		client->flags &= ~I2C_CLIENT_PEC;
> -	else
> -		client->flags |= I2C_CLIENT_PEC;
> -
>  	err = count;
>  put:
>  	put_device(hdev);

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