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

From: Guenter Roeck
Date: Wed Aug 11 2021 - 13:52:07 EST


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@xxxxxxxxx>