Re: [PATCH v2] power: supply: bq27xxx: Fix kernel crash on IRQ handler register error

[Hans de Goede <hdegoede@redhat.com>]

Hi Sebastian,

On 11/2/21 14:23, Sebastian Reichel wrote:
> Hi,
> 
> On Sun, Oct 31, 2021 at 09:34:46PM +0200, Andy Shevchenko wrote:
>> On Sun, Oct 31, 2021 at 5:25 PM Hans de Goede <hdegoede@redhat.com> wrote:
>>>
>>> When registering the IRQ handler fails, do not just return the error code,
>>> this will free the devm_kzalloc()-ed data struct while leaving the queued
>>> work queued and the registered power_supply registered with both of them
>>> now pointing to free-ed memory, resulting in various kernel crashes
>>> soon afterwards.
>>>
>>> Instead properly tear-down things on IRQ handler register errors.
>>
>> FWIW,
>> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> 
> Thanks, queued.

Thank you, note these 2 patches (for the bq25980 driver) are also
pure bug-fixes, I posted them as part of a larger series, but
in hindsight I should have probably posted them separately:

https://lore.kernel.org/platform-driver-x86/20211030182813.116672-4-hdegoede@redhat.com/
https://lore.kernel.org/platform-driver-x86/20211030182813.116672-5-hdegoede@redhat.com/

It would be good if you can pick these 2 up too
(I'll respin the rest of the series to address various
review comments without them then).

Let me know if you want me to resend these 2 as a stand alone
series.

Regards,

Hans



>>> Fixes: 703df6c09795 ("power: bq27xxx_battery: Reorganize I2C into a module")
>>> Cc: Andrew F. Davis <afd@ti.com>
>>> Signed-off-by: Hans de Goede <hdegoede@redhat.com>
>>> ---
>>> Changes in v2:
>>> - Fix devm_kzalloc()-ed type in the commit message
>>> ---
>>>  drivers/power/supply/bq27xxx_battery_i2c.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/power/supply/bq27xxx_battery_i2c.c b/drivers/power/supply/bq27xxx_battery_i2c.c
>>> index 46f078350fd3..cf38cbfe13e9 100644
>>> --- a/drivers/power/supply/bq27xxx_battery_i2c.c
>>> +++ b/drivers/power/supply/bq27xxx_battery_i2c.c
>>> @@ -187,7 +187,8 @@ static int bq27xxx_battery_i2c_probe(struct i2c_client *client,
>>>                         dev_err(&client->dev,
>>>                                 "Unable to register IRQ %d error %d\n",
>>>                                 client->irq, ret);
>>> -                       return ret;
>>> +                       bq27xxx_battery_teardown(di);
>>> +                       goto err_failed;
>>>                 }
>>>         }
>>>
>>> --
>>> 2.31.1
>>>
>>
>>
>> -- 
>> With Best Regards,
>> Andy Shevchenko