Re: [BUG] hwmon: Widespread TOCTOU vulnerabilities in the hwmon subsystem

[Armin Wolf]

Am 07.06.25 um 01:20 schrieb Guenter Roeck:

> On 6/6/25 14:30, Armin Wolf wrote:
> > Am 06.06.25 um 09:03 schrieb Gui-Dong Han:
> >
> > > > On Thu, Jun 05, 2025 at 07:33:24AM -0700, Guenter Roeck wrote:
> > > > > > I would like to discuss these issues further and collaborate on the
> > > > > > best way to address them comprehensively.
> > > > > I'd suggest to start submitting patches, with the goal of minimizing
> > > > > the scope of changes. Sometimes that may mean expanding the scope of
> > > > > locks, sometimes it may mean converting macros to functions. When
> > > > > converting to functions, it doesn't have to be inline functions: I'd
> > > > > leave that up to the compiler to decide. None of that code is 
> > > > > performance
> > > > > critical.
> > > > Actualy, that makes me wonder if it would make sense to introduce
> > > > subsystem-level locking. We could introduce a lock in struct
> > > > hwmon_device_attribute and lock it whenever a show or store function
> > > > executes in drivers/hwmon/hwmon.c. That would only help for drivers
> > > > using the _with_info API, but it would simplify driver code a lot.
> > > > Any thoughts on that ?
> >
> > Hi,
> >
> > i am against adding a subsystem lock just to work around buggy 
> > drivers. Many drivers
> > should use fine-grained locking to avoid high latencies when reading 
> > a single attribute.
>
> The point would be driver code simplification and increasing 
> robustness, not
> working around buggy drivers.
>
> Anyway, what high latency are you talking about ? Serialization of 
> attribute
> accesses would only increase latency if multiple processes read 
> attributes from
> the same driver at the same time, which is hardly a typical use case.
>
> Guenter

Some drivers read all registers (fan, temperature, etc) when updating the readings, and depending
on the underlying bus system this might take some time. With a global lock reading a single value will thus
take as much time as reading all values.

Thanks,
Armin Wolf