Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

phy: ti: tusb1210: Resolve charger-det crash if charger psy is unregistered

The power_supply frame-work is not really designed for there to be
long living in kernel references to power_supply devices.

Specifically unregistering a power_supply while some other code has
a reference to it triggers a WARN in power_supply_unregister():

WARN_ON(atomic_dec_return(&psy->use_cnt));

Folllowed by the power_supply still getting removed and the
backing data freed anyway, leaving the tusb1210 charger-detect code
with a dangling reference, resulting in a crash the next time
tusb1210_get_online() is called.

Fix this by only holding the reference in tusb1210_get_online()
freeing it at the end of the function. Note this still leaves
a theoretical race window, but it avoids the issue when manually
rmmod-ing the charger chip driver during development.

Fixes: 48969a5623ed ("phy: ti: tusb1210: Add charger detection")
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Link: https://lore.kernel.org/r/20240406140821.18624-1-hdegoede@redhat.com
Signed-off-by: Vinod Koul <vkoul@kernel.org>

authored by

Hans de Goede and committed by
Vinod Koul
bf6e4ee5 025a6f74

+12 -11
+12 -11
drivers/phy/ti/phy-tusb1210.c
··· 69 69 struct delayed_work chg_det_work; 70 70 struct notifier_block psy_nb; 71 71 struct power_supply *psy; 72 - struct power_supply *charger; 73 72 #endif 74 73 }; 75 74 ··· 235 236 236 237 static bool tusb1210_get_online(struct tusb1210 *tusb) 237 238 { 239 + struct power_supply *charger = NULL; 238 240 union power_supply_propval val; 239 - int i; 241 + bool online = false; 242 + int i, ret; 240 243 241 - for (i = 0; i < ARRAY_SIZE(tusb1210_chargers) && !tusb->charger; i++) 242 - tusb->charger = power_supply_get_by_name(tusb1210_chargers[i]); 244 + for (i = 0; i < ARRAY_SIZE(tusb1210_chargers) && !charger; i++) 245 + charger = power_supply_get_by_name(tusb1210_chargers[i]); 243 246 244 - if (!tusb->charger) 247 + if (!charger) 245 248 return false; 246 249 247 - if (power_supply_get_property(tusb->charger, POWER_SUPPLY_PROP_ONLINE, &val)) 248 - return false; 250 + ret = power_supply_get_property(charger, POWER_SUPPLY_PROP_ONLINE, &val); 251 + if (ret == 0) 252 + online = val.intval; 249 253 250 - return val.intval; 254 + power_supply_put(charger); 255 + 256 + return online; 251 257 } 252 258 253 259 static void tusb1210_chg_det_work(struct work_struct *work) ··· 477 473 cancel_delayed_work_sync(&tusb->chg_det_work); 478 474 power_supply_unregister(tusb->psy); 479 475 } 480 - 481 - if (tusb->charger) 482 - power_supply_put(tusb->charger); 483 476 } 484 477 #else 485 478 static void tusb1210_probe_charger_detect(struct tusb1210 *tusb) { }