Re: [PATCH v2] hwmon: (lm63) Add locking to avoid TOCTOU

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [High] temp2_crit_hyst_store() relies on uninitialized cached data if invoked before lm63_update_device()
--

commit 0ac9742e5a731c1457b701e413708de120dd3347
Author: Gui-Dong Han <hanguidong02@gmail.com>

hwmon: (lm63) Add locking to avoid TOCTOU

This patch adds locking to several show functions to prevent TOCTOU races
when accessing shared cached data, ensuring values remain stable during
calculations.

Link: https://lore.kernel.org/linux-hwmon/CALbr=LYJ_ehtp53HXEVkSpYoub+XYSTU8Rg=o1xxMJ8=5z8B-g@mail.gmail.com/

> diff --git a/drivers/hwmon/lm63.c b/drivers/hwmon/lm63.c
> index 035176a98ce9c..b8b1de5fa90f1 100644
> --- a/drivers/hwmon/lm63.c
> +++ b/drivers/hwmon/lm63.c

[ ... ]

> @@ -592,9 +604,14 @@ static ssize_t temp2_crit_hyst_show(struct device *dev,
>  				    struct device_attribute *dummy, char *buf)
>  {
>  	struct lm63_data *data = lm63_update_device(dev);
> -	return sprintf(buf, "%d\n", temp8_from_reg(data, 2)
> -		       + data->temp2_offset
> -		       - TEMP8_FROM_REG(data->temp2_crit_hyst));
> +	int temp;
> +
> +	mutex_lock(&data->update_lock);
> +	temp = temp8_from_reg(data, 2) + data->temp2_offset
> +	     - TEMP8_FROM_REG(data->temp2_crit_hyst);
> +	mutex_unlock(&data->update_lock);
> +
> +	return sprintf(buf, "%d\n", temp);
>  }

This isn't a bug introduced by this patch, but looking at the corresponding
store function, is there a pre-existing initialization issue?

static ssize_t temp2_crit_hyst_store(...)
{
        struct lm63_data *data = dev_get_drvdata(dev);
        ...
        mutex_lock(&data->update_lock);
        hyst = temp8_from_reg(data, 2) + data->temp2_offset - val;

Unlike the show functions, temp2_crit_hyst_store() retrieves the context
via dev_get_drvdata(dev) rather than calling lm63_update_device(dev).

If it is invoked before any show function (for example, by an init script
restoring settings on boot), will data->temp8[2] still be 0 due to the
initial devm_kzalloc()?

Could this cause the relative hysteresis to be computed against 0 instead
of the actual hardware default critical temperature, resulting in the
hardware's hysteresis register being misprogrammed?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260416135703.53262-1-hanguidong02@gmail.com?part=1