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?
The same situation in w83791d.c and in w83792d.
Found by Linux Driver Verification project (linuxtesting.org).
Signed-off-by: Nadezda Lutovinova <[email protected]>
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 <[email protected]>
On Wed, Aug 11, 2021 at 10:52:03AM -0700, Guenter Roeck wrote:
> 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.
Wait, that is wrong. Those are just the initial register values.
Forget the noise above; sorry for the confusion.
Guenter
> 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 <[email protected]>