| Message ID | 20230415160734.70475-1-hdegoede@redhat.com |
|---|---|
| Headers | show |
| Series | power: supply: Fix external_power_changed race in several drivers | expand |
Hi Hans, On Sat, Apr 15, 2023 at 06:07:28PM +0200, Hans de Goede wrote: > I hit this issue where a Lenovo Yoga Book yb1-x90f would hang on boot > every couple of boots. > > The problem was that bq25890_charger_external_power_changed() dereferences > bq->charger, which gets sets in bq25890_power_supply_init() like this: > > bq->charger = devm_power_supply_register(bq->dev, &bq->desc, &psy_cfg); > > As soon as devm_power_supply_register() has called device_add() > the external_power_changed callback can get called. So there is a window > where bq25890_charger_external_power_changed() may get called while > bq->charger has not been set yet leading to a NULL pointer dereference. > > This race hits on the yb1-x90f when the cht_wcove_pwrsrc (extcon) psy is > done with detecting the charger, which happens to exactly hit the window: > > BUG: kernel NULL pointer dereference, address: 0000000000000018 > <snip> > RIP: 0010:__power_supply_is_supplied_by+0xb/0xb0 > <snip> > Call Trace: > <TASK> > __power_supply_get_supplier_property+0x19/0x50 > class_for_each_device+0xb1/0xe0 > power_supply_get_property_from_supplier+0x2e/0x50 > bq25890_charger_external_power_changed+0x38/0x1b0 [bq25890_charger] > __power_supply_changed_work+0x30/0x40 > class_for_each_device+0xb1/0xe0 > power_supply_changed_work+0x5f/0xe0 > <snip> > > This series fixes this issue in the bq25890 + 3 other drivers with > the same issue. Patches 5-6 are small cleanup patches for 2 other > drivers which I audited for the same issue. Thanks, I queued the first 4 patches to my fixes branch and the 2 cleanups to for-next. -- Sebastian > Hans de Goede (6): > power: supply: ab8500: Fix external_power_changed race > power: supply: axp288_fuel_gauge: Fix external_power_changed race > power: supply: bq25890: Fix external_power_changed race > power: supply: sc27xx: Fix external_power_changed race > power: supply: max17042_battery: Refactor > max17042_external_power_changed() > power: supply: twl4030_madc_battery: Refactor > twl4030_madc_bat_ext_changed() > > drivers/power/supply/ab8500_btemp.c | 6 ++---- > drivers/power/supply/ab8500_fg.c | 6 ++---- > drivers/power/supply/axp288_fuel_gauge.c | 2 +- > drivers/power/supply/bq25890_charger.c | 2 +- > drivers/power/supply/max17042_battery.c | 7 +------ > drivers/power/supply/sc27xx_fuel_gauge.c | 9 +-------- > drivers/power/supply/twl4030_madc_battery.c | 8 +------- > 7 files changed, 9 insertions(+), 31 deletions(-) > > -- > 2.39.1 >
Hi All, I hit this issue where a Lenovo Yoga Book yb1-x90f would hang on boot every couple of boots. The problem was that bq25890_charger_external_power_changed() dereferences bq->charger, which gets sets in bq25890_power_supply_init() like this: bq->charger = devm_power_supply_register(bq->dev, &bq->desc, &psy_cfg); As soon as devm_power_supply_register() has called device_add() the external_power_changed callback can get called. So there is a window where bq25890_charger_external_power_changed() may get called while bq->charger has not been set yet leading to a NULL pointer dereference. This race hits on the yb1-x90f when the cht_wcove_pwrsrc (extcon) psy is done with detecting the charger, which happens to exactly hit the window: BUG: kernel NULL pointer dereference, address: 0000000000000018 <snip> RIP: 0010:__power_supply_is_supplied_by+0xb/0xb0 <snip> Call Trace: <TASK> __power_supply_get_supplier_property+0x19/0x50 class_for_each_device+0xb1/0xe0 power_supply_get_property_from_supplier+0x2e/0x50 bq25890_charger_external_power_changed+0x38/0x1b0 [bq25890_charger] __power_supply_changed_work+0x30/0x40 class_for_each_device+0xb1/0xe0 power_supply_changed_work+0x5f/0xe0 <snip> This series fixes this issue in the bq25890 + 3 other drivers with the same issue. Patches 5-6 are small cleanup patches for 2 other drivers which I audited for the same issue. Regards, Hans Hans de Goede (6): power: supply: ab8500: Fix external_power_changed race power: supply: axp288_fuel_gauge: Fix external_power_changed race power: supply: bq25890: Fix external_power_changed race power: supply: sc27xx: Fix external_power_changed race power: supply: max17042_battery: Refactor max17042_external_power_changed() power: supply: twl4030_madc_battery: Refactor twl4030_madc_bat_ext_changed() drivers/power/supply/ab8500_btemp.c | 6 ++---- drivers/power/supply/ab8500_fg.c | 6 ++---- drivers/power/supply/axp288_fuel_gauge.c | 2 +- drivers/power/supply/bq25890_charger.c | 2 +- drivers/power/supply/max17042_battery.c | 7 +------ drivers/power/supply/sc27xx_fuel_gauge.c | 9 +-------- drivers/power/supply/twl4030_madc_battery.c | 8 +------- 7 files changed, 9 insertions(+), 31 deletions(-)