Re: [PATCH] hwmon: (w83l786ng) Convert macros to functions to avoid TOCTOU

[Gui-Dong Han]

On Sat, Nov 29, 2025 at 3:37 AM david laight <david.laight@xxxxxxxxxx> wrote:
>
> On Fri, 28 Nov 2025 20:38:16 +0800
> Gui-Dong Han <hanguidong02@xxxxxxxxx> wrote:
>
> > The macros FAN_FROM_REG and TEMP_FROM_REG evaluate their arguments
> > multiple times. When used in lockless contexts involving shared driver
> > data, this causes Time-of-Check to Time-of-Use (TOCTOU) race
> > conditions.
> >
> > Convert the macros to static functions. This guarantees that arguments
> > are evaluated only once (pass-by-value), preventing the race
> > conditions.
> >
> > Adhere to the principle of minimal changes by only converting macros
> > that evaluate arguments multiple times and are used in lockless
> > contexts.
> >
> > Link: https://lore.kernel.org/all/CALbr=LYJ_ehtp53HXEVkSpYoub+XYSTU8Rg=o1xxMJ8=5z8B-g@xxxxxxxxxxxxxx/
> > Fixes: 85f03bccd6e0 ("hwmon: Add support for Winbond W83L786NG/NR")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Gui-Dong Han <hanguidong02@xxxxxxxxx>
> > ---
> > Based on the discussion in the link, I will submit a series of patches to
> > address TOCTOU issues in the hwmon subsystem by converting macros to
> > functions or adjusting locking where appropriate.
> > ---
> >  drivers/hwmon/w83l786ng.c | 26 ++++++++++++++++++--------
> >  1 file changed, 18 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/hwmon/w83l786ng.c b/drivers/hwmon/w83l786ng.c
> > index 9b81bd406e05..1d9109ca1585 100644
> > --- a/drivers/hwmon/w83l786ng.c
> > +++ b/drivers/hwmon/w83l786ng.c
> > @@ -76,15 +76,25 @@ FAN_TO_REG(long rpm, int div)
> >       return clamp_val((1350000 + rpm * div / 2) / (rpm * div), 1, 254);
> >  }
> >
> > -#define FAN_FROM_REG(val, div)       ((val) == 0   ? -1 : \
> > -                             ((val) == 255 ? 0 : \
> > -                             1350000 / ((val) * (div))))
> > +static int fan_from_reg(int val, int div)
> > +{
> > +     if (val == 0)
> > +             return -1;
> > +     if (val == 255)
> > +             return 0;
> > +     return 1350000 / (val * div);
> > +}
> >
> >  /* for temp */
> >  #define TEMP_TO_REG(val)     (clamp_val(((val) < 0 ? (val) + 0x100 * 1000 \
> >                                                     : (val)) / 1000, 0, 0xff))
>
> Can you change TEMP_TO_REG() as well.
> And just use plain clamp() while you are at it.
> Both these temperature conversion functions have to work with negative temperatures.
> But the signed-ness gets passed through from the parameter - which may not be right.
> IIRC some come from FIELD_GET() and will be 'unsigned long' unless cast somewhere.
> The function parameter 'corrects' the type to a signed one.
>
> So you are fixing potential bugs as well.

Hi David,

Thanks for your feedback on TEMP_TO_REG and the detailed explanation
regarding macro risks.

Guenter has already applied this patch. Since the primary scope here
was strictly addressing TOCTOU race conditions (and TEMP_TO_REG is not
used in lockless contexts), it wasn't included.

However, I appreciate your point regarding type safety. I will look
into addressing that in a future separate patch.

Best regards,
Gui-Dong Han