linux-kernel - Re: [PATCH v2] hwmon: (asus-ec-sensors) add WARN

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1e5fc32c-04dd-46b1-8943-03fd9370bfdc@roeck-us.net>
Date: Thu, 1 May 2025 05:34:43 -0700
From: Guenter Roeck <linux@...ck-us.net>
To: Eugene Shalygin <eugene.shalygin@...il.com>
Cc: Alexei Safin <a.safin@...a.ru>, Jean Delvare <jdelvare@...e.com>,
 linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org,
 lvc-project@...uxtesting.org
Subject: Re: [PATCH v2] hwmon: (asus-ec-sensors) add WARN_ONCE() on invalid
 sensor index

On 5/1/25 01:07, Eugene Shalygin wrote:
> On Wed, 30 Apr 2025 at 06:13, Guenter Roeck <linux@...ck-us.net> wrote:
> 
>>> Thanks for the explanation! I'm still not convinced that such a
>>> generic error message (without the type and channel) can be of great
>>
>> Feel free to suggest a better one. Maybe I misunderstood your earlier
>> concerns, but it seemed to me that you objected to having a message
>> in the first place, not to its contents.
> 
> The only two conditions I can imaging to trigger the log message are
> hardware and/or firmware change and RAM instability. In both cases the
> message is not helpful: for the hardware change case I would need
> sensor type and channel, for the RAM-related cases I would need to
> know how often the problem repeats itself. Neither is possible with
> this log message, and therefore I'd rather log every time and with
> sensor type and channel.
> 

The message would only be expected if there is some programming error,
where is_visible does not correctly disable non-existing sensors.
That is the situation the messages try to catch.

You do have an excellent point, though: The messages should display the
sensor type and channel. As for logging it every time: No, because _if_
there is a programming error it would clog the log, so once is enough
to tell "something is wrong with the code, fix it". The same is actually
true if the hardware changes "under the hood".

Regarding RAM errors: Those won't be caught by checks like this. You'd have
to litter the whole kernel with checks at almost every line of code
to even have a remote chance to catch problems like bit flips. Even
then you'd still not catch cases where the code itself is changed.
Even trying to catch that would be futile.

Thanks,
Guenter