linux-kernel - Re: [PATCH v2] hwmon: add HP WMI Sensors driver

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: <e256d281-49a5-2d9a-7def-fd68e177e926@roeck-us.net>
Date:   Fri, 7 Apr 2023 05:54:48 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     James Seo <james@...iv.tech>, Armin Wolf <W_Armin@....de>
Cc:     Jean Delvare <jdelvare@...e.com>, linux-hwmon@...r.kernel.org,
        platform-driver-x86@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] hwmon: add HP WMI Sensors driver

On 4/6/23 22:39, James Seo wrote:
> Hi,
> 
>> is it guaranteed that faulty sensors wont become operational later?
>> Also filtering out such sensors would make the support for the hwmon_temp_fault and
>> hwmon_fan_fault attributes meaningless.
> 
> Good point. I can't be certain, but the MOF does seem to imply that
> sensors can indeed be faulty on just a temporary basis.
> 

Your current code would explicitly exclude faulty fans from being listed,
which does not exactly sound like a good idea.

> I'll filter out only the sensors that are "Not Connected" at probe
> time. My thinking is, even if these might turn into connected sensors
> later, that would mean the user is e.g. hot-plugging a fan (!), and
> keeping them could result in a large number (~10 on my Z420) of
> pointless extra channels. And this would also match the behavior of
> HP's official utility.
> 
Ultimately that is an implementation decision. Are the sensors hot-pluggable ?
If so, how does HP's utility handle the insertion or removal of a sensor (fan) ?

Either case, it is ok with me if disconnected sensors are not listed.
Not listing faulty sensors seems like a bad idea, though.

Guenter

> Does that seem reasonable? Or did you mean that I shouldn't filter,
> and leave disconnected sensors in like some other hwmon drivers do?
> 
>> The sanity check for HP_WMI_NUMERIC_SENSOR_GUID is unnecessary, the WMI driver core already makes sure that your driver
>> is only matched with WMI devices containing HP_WMI_NUMERIC_SENSOR_GUID.
>> As for the sanity check regarding HP_WMI_BIOS_GUID: this WMI GUID is not used inside the driver. Since WMI GUIDs are expected
>> to be unique, checking for HP_WMI_BIOS_GUID (which AFAIK is used by the HP-BIOSCFG driver) without intending to use it is
>> meaningless.
> 
> In that case, I'll gladly remove the checks. I was following the
> example of the platform/x86/hp-wmi driver, which checks for that GUID
> and another at module load.
> 
> Thanks for reviewing.
> 
> James