linux-kernel - Re: hwmon: Error handling in w83793.c, w83791d.c, w83792d.c

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: <20210811175120.GA3138792@roeck-us.net>
Date:   Wed, 11 Aug 2021 10:51:20 -0700
From:   Guenter Roeck <linux@...ck-us.net>
To:     Nadezda Lutovinova <lutovinova@...ras.ru>
Cc:     Marc Hulsman <m.hulsman@...elft.nl>,
        Jean Delvare <jdelvare@...e.com>,
        Rudolf Marek <r.marek@...embler.cz>,
        linux-hwmon@...r.kernel.org, linux-kernel@...r.kernel.org,
        ldv-project@...uxtesting.org
Subject: Re: hwmon: Error handling in w83793.c, w83791d.c, w83792d.c

On Wed, Aug 11, 2021 at 07:15:14PM +0300, Nadezda Lutovinova wrote:
> In w83793_detect_subclients(): if driver read tmp value sufficient for 
> (tmp & 0x08) && (!(tmp & 0x80)) && ((tmp & 0x7) == ((tmp >> 4) & 0x7))
> from device then Null pointer dereference occurs.
> (It is possible if tmp = 0b0xyz1xyz, where same chars mean same numbers).
> 
> It can be fixed just by adding a checking for null pointer, patch for 
> this is in the next letter. But a question arised:
> The array w83793_data->lm75 is used once in this function after switching 
> to devm_i2c_new_dummy_device() instead of i2c_new_dummy(). And this 
> function is called once (from w83793_probe()). Maybe this array should be 
> deleted from struct w83793_data?
> 

That is part of it. However, the entire code is wrong. There is no need
to check for I2C address overlap in the first place, and the i2c address
for the second 'virtual' chip should start with 0x4c, not with 0x48.
See w83793g/w83793ag datasheet, section 8.3.2.2.
I assume the code was copied from w83791d and w83792d, where the addresses
can indeed overlap.

Guenter

> The same situation in w83791d.c and in w83792d.
> 
> Found by Linux Driver Verification project (linuxtesting.org).
> 
> Signed-off-by: Nadezda Lutovinova <lutovinova@...ras.ru>