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

[Armin Wolf <W_Armin@gmx.de>]

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.

Thanks,
Armin Wolf

> Hi Guenter,
>
> Thanks for your quick and insightful feedback!
>
> I agree with your suggestion. Adding a note to
> Documentation/hwmon/submitting-patches.rst about avoiding calculations
> in macros is also a great idea to prevent this class of bugs in the
> future.
>
> Regarding your thoughts on subsystem-level locking, it sounds like a
> promising approach to simplify the drivers using the _with_info API.
> As you mentioned, some drivers don't use this API, so they would still
> require manual fixes.
>
> For the subsystem-level lock itself, I was wondering if a read-write
> semaphore might be more appropriate than a standard mutex. This would
> prevent a single show operation from blocking other concurrent reads.
> I'm not entirely sure about all the implications, but it might be
> worth considering to maintain read performance.
>
> Thanks again for your guidance.
>
> Best regards,
> Gui-Dong Han
>