linux-kernel - Re: [PATCH] hwmon: lm75: Use zero lm75

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 for Android: free password hash cracker in your pocket

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

Date:   Sat, 30 Jan 2021 08:00:07 -0800
From:   Guenter Roeck <linux@...ck-us.net>
To:     "Matwey V. Kornilov" <matwey.kornilov@...il.com>
Cc:     Jean Delvare <jdelvare@...e.com>,
        Javier Martinez Canillas <javier@....samsung.com>,
        "open list:HARDWARE MONITORING" <linux-hwmon@...r.kernel.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] hwmon: lm75: Use zero lm75_type for lm75

On 1/30/21 7:43 AM, Matwey V. Kornilov wrote:
> 
> 
> сб, 30 янв. 2021 г. в 18:31, Guenter Roeck <linux@...ck-us.net <mailto:linux@...ck-us.net>>:
>>
>> On 1/30/21 2:10 AM, Matwey V. Kornilov wrote:
>> > There is a logical flaw in lm75_probe() function introduced in
>> >
>> >     e97a45f1b460 ("hwmon: (lm75) Add OF device ID table")
>> >
>> > Note, that of_device_get_match_data() returns NULL when no match
>> > found. This is the case when OF node exists but has unknown
>> > compatible line, while the module is still loaded via i2c
>> > detection.
>> >
>> > arch/powerpc/boot/dts/fsl/p2041rdb.dts:
>> >
>> >     lm75b@48 {
>> >       compatible = "nxp,lm75a";
>> >       reg = <0x48>;
>> >     };
>> >
>> > In this case, the sensor is mistakenly considered as ADT75 variant.
>> > The simplest way to handle this issue is to make the LM75 code
>> > zero.
>> >
>>
>> This doesn't really solve the problem since it would match _all_
>> non-existing entries with lm75 (instead of whatever is intended).
> 
> Just exactly how it happened before e97a45f1b460
> 
>> That doesn't matter for lm75a, but it would matter if someone
>> would enter, say, "bla,adt75".
>>
>> On a side note, "nxp,lm75a" (nor "nxp,lm75", for that matter) is not a
>> documented compatible string for this driver. If anything, we would
>> need a means to explicitly reject such undefined compatible strings.
>> One option might be to define the first entry in enum lm75_type
>> explicitly as invalid, check for it and reject it if returned.
> 
> It is fine for me. I am afraid that this will break some dts files in the tree.
> The following compatible strings missed in the driver are currently in use:
> 
> ti,lm75
> nxp,lm75
> nxp,lm75a
> 
> I suppose these boards currently rely on the i2c detection path.
> 

Correct. But relying on a bug doesn't improve the situation.
The above compatible strings need to be documented (and properly
implemented), or removed. And we really need a better means to handle
NULL returns from of_device_get_match_data(). Maybe we should use
of_match_device() instead and check for a NULL return.

Guenter