power: supply: bq24190: IRQ handler can call power_supply_changed after the psy is unregistered

[徐晗翔]

bq24190_remove() unregisters the battery and charger power_supplies
before pm_runtime_disable(); the IRQ is requested with devm and is
therefore freed only after remove returns.  A threaded IRQ handler that
is already running, or one that runs in the window between the two
unregisters and pm_runtime_disable(), calls bq24190_check_status() ->
power_supply_changed() on a psy that has already been unregistered (and
may be freed).  This is a driver remove-order bug; the window is
narrower than bq25890 but real.

The detailed description:

* Test environment / affected version

  Linux 7.3.0 source tree (unmodified kernel.org 7.3), x86_64.  This is
  a source-level report.  I did not run it on bq24190 hardware: the
  KASAN build used for my core tests does not enable
  CONFIG_CHARGER_BQ24190 and has no I2C bq24190 device.  No reproducer
  is attached.  Line numbers are from 7.3.

* Mechanism

  Probe:

      :2121  power_supply_register(bdi->battery, ...)
      :2133  power_supply_register(bdi->charger, ...)
      :2160  devm_request_threaded_irq(..., bq24190_irq_handler_thread, ...)

  Both power_supplies are non-devm.  The IRQ is devm, so it is released
  by devres after bq24190_remove() returns, not before.

  bq24190_remove() (:2189-2206):

      :2200  power_supply_unregister(bdi->battery);
      :2201  power_supply_unregister(bdi->charger);
      :2202-2204  pm_runtime_put...
      :2205  pm_runtime_disable...

  The threaded handler (:1834-1850) calls bq24190_check_status()
  (:1824-1829), which calls power_supply_changed(bdi->charger/battery).

  Window A: an IRQ is delivered after :2201 and before :2205; the
  handler runs while both psy are already unregistered.  Window B: a
  handler is already running when :2200/:2201 execute; it can reach
  power_supply_changed() after unregister.  After :2205,
  pm_runtime_disable() makes new IRQ handling less likely to reach the
  callback, so I do not claim arbitrary post-remove IRQs; the
  deterministic window is :2201-:2205, plus the in-flight handler.

* Concrete corruption / impact

  power_supply_changed() dereferences psy->changed_lock; if the psy has
  been freed, this is a slab-use-after-free and can corrupt the
  workqueue or crash.  If a consumer reference keeps the psy alive, the
  call is a use-after-unregister and can reach the freed LED triggers
  described in the core 5B report on a 7.3 kernel without the
  9f7f7a859451 get/set guard.  I have not demonstrated privilege
  escalation, and I have not run this on hardware.

* Trigger model

  bq24190 with a charger interrupt (AC plug/unplug, fault, or charger
  status change) concurrent with unbind/remove.  The window is narrow
  (between the two unregisters and pm_runtime_disable, or an in-flight
  handler), so it is a race rather than a deterministic sequence.
  Unbinding requires root; a physical charger event is the natural
  trigger.  No generic unprivileged userspace interface.

* Reproducer / results

  No reproducer attached.  This is a source-level report; I can test a
  proposed fix if bq24190 hardware is available.  The relevant call
  chain is fully visible in 7.3 at the line numbers above.


* Suggested direction 

  Disable the IRQ and wait for an in-flight handler before unregistering
  the power_supplies: call disable_irq()/free_irq() (or an explicit
  disable_irq + synchronize_irq) before :2200, and move
  pm_runtime_disable() before the unregisters.  Alternatively have the
  handler check a "removed" flag set under a lock before
  power_supply_changed().  Report-first, no patch attached; happy to
  test a proposal.


  Please keep me in Cc; I am not subscribed to the lists.


Regards,
Hanxiang Xu
xuhx@xxxxxxxxxxx