power: supply: core: changed work can run after unregister and use freed LED triggers

[徐晗翔 <xuhx@hust.edu.cn>]

[-- Attachment #1: Type: text/plain, Size: 6156 bytes --]

power_supply_changed() has no removing check and always schedules
psy->changed_work.  power_supply_unregister() sets removing and calls
cancel_work_sync() once, but a driver can call power_supply_changed()
after that point (from its own work or IRQ) and requeue the work.
changed_work then runs after power_supply_remove_triggers() has freed
the LED triggers; the trigger pointers are not cleared, so update_leds()
writes through freed memory.  KASAN reproduced the trigger UAF on 7.3.

This is a 7.3 report.  The 9f7f7a859451 guard on the get/set wrappers is
not an explicit changed_work fix, but it masks the LED dereference below
because update_leds() first calls power_supply_get_property(); on a
kernel with 9f7f that call returns -ENODEV and the trigger write is
skipped.  changed_work still runs after unregister and the trigger
pointers are still dangling, so I report this as 7.3 exposure and as an
explicit changed_work/trigger cleanup gap, not as a new mainline UAF.

The detailed description:

* Test environment / affected version

  Linux 7.3.0 (unmodified kernel.org 7.3 source tree), gcc 13.3.0,
  x86_64, SMP PREEMPT_DYNAMIC, KASAN, QEMU/KVM.
  CONFIG_POWER_SUPPLY=y, CONFIG_POWER_SUPPLY_HWMON=y,
  CONFIG_LEDS_TRIGGERS=y, CONFIG_MODULES=y.  No source changes.
  The 7.3 tree I tested has no removing check in power_supply_changed()
  and no NULL store in power_supply_remove_triggers().  I did not run a
  9f7f-patched kernel; the masking statement above follows from the
  guard in power_supply_get_property() and the first call in
  update_leds().

* Mechanism

  power_supply_changed() (:150-162):

      spin_lock_irqsave(&psy->changed_lock, flags);
      psy->changed = true;
      pm_stay_awake(&psy->dev);
      spin_unlock_irqrestore(&psy->changed_lock, flags);
      schedule_work(&psy->changed_work);

  There is no psy->removing check.

  unregister (:1928-1951):

      WARN_ON(atomic_dec_return(&psy->use_cnt));
      scoped_guard(rwsem_write, &psy->epc_sem)
          psy->removing = true;
      cancel_work_sync(&psy->changed_work);
      cancel_delayed_work_sync(&psy->deferred_register_work);
      ...
      power_supply_remove_triggers(psy);
      ...
      device_unregister(&psy->dev);

  The cancel only cancels work queued before the call.  Any
  power_supply_changed() after :1942 queues it again; changed_work has
  no removing check either (:78-124).

  power_supply_remove_triggers() (power_supply_leds.c:207-215)
  unregisters and frees the online/status/etc triggers but does not set
  psy->online_trig/psy->status_trig to NULL.  update_online_leds()
  (:157-167) calls led_trigger_event(psy->online_trig, ...) after a
  successful get_property(ONLINE); update_status_leds() (:90-134) does
  the same for status.  A consumer reference can keep the struct
  power_supply alive, but it does not keep the triggers alive: they are
  freed by unregister.  changed_work then writes to freed trigger
  memory.

  If the caller has no reference, the struct power_supply itself can be
  freed first; that is the driver-side 5A variant (bq25890/bq24190).
  This report is the core-side 5B variant: the psy is still referenced,
  but the triggers are gone.

* Concrete corruption / impact

  KASAN reports a slab-use-after-free write in led_trigger_event() from
  power_supply_changed_work:

      BUG: KASAN: slab-use-after-free in led_trigger_event+0x1f/0x80
      Write of size 4 at addr ff1100000368b918
      ...
      BUG: KASAN: slab-use-after-free in led_trigger_event+0x2f/0x80
      Read of size 8

  The freed object is the LED trigger allocated by
  power_supply_register_led_trigger() and freed by
  power_supply_remove_triggers().  The write can corrupt the LED
  trigger list or the LED class device; at minimum it is a KASAN
  splat/panic.  I have not demonstrated privilege escalation.

* Trigger model

  A consumer must hold a power_supply reference (so use_cnt > 0 and the
  get_property(ONLINE) in update_leds() can succeed), and a driver must
  call power_supply_changed() after unregister.  On 7.3 without the 9f7f
  guard this reaches the freed trigger; with 9f7f the trigger write is
  skipped and the residual issue is changed_work running after
  unregister.  In-tree asynchronous
  callers include bq25890's pump work and bq24190's threaded IRQ; both
  are reported separately as 5A because they can also free the psy
  itself.  The core path is generic and can be exercised by any driver
  that calls changed from a work/IRQ without a remove-order guarantee.
  The test module is loaded by root; real triggers are unbind/remove or
  hardware events, not a generic unprivileged interface.

* Reproducer / results

  psrace.c (attached), plan=ps5.  The module registers a USB
  psy with ONLINE, keeps a get_by_name() reference, unregisters the psy
  (freeing the online trigger), calls power_supply_changed(), sleeps for
  changed_work, and finally puts the reference.

  Observed on 7.3:

      online_trig=ff1100000368b900
      held=ff11000003707000
      unregister
      WARNING: ... power_supply_core.c:1930 ...
      after unregister online_trig=ff1100000368b900
      BUG: KASAN: slab-use-after-free in led_trigger_event+0x1f/0x80
      Write of size 4 at addr ff1100000368b918
      task kworker/3:0/34, Workqueue: power_supply_changed_work

  A control run (control=1, no changed after unregister) reports no
  KASAN.  Full logs are available on request.



* Suggested direction

  If 9f7f is backported, the LED dereference is already masked; the
  remaining cleanup is still worth doing.  Under psy->changed_lock, have
  power_supply_changed() reject psy->removing; set removing under the
  same lock in unregister before cancel_work_sync(); and recheck
  removing at the top of power_supply_changed_work().  Also set the
  trigger pointers to NULL in power_supply_remove_triggers() and make
  update_leds() tolerate NULL.  This is defense in depth; drivers must
  still quiesce their async callers before unregister.  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@hust.edu.cn

[-- Attachment #2: psrace.c --]
[-- Type: text/plain, Size: 9089 bytes --]

// SPDX-License-Identifier: GPL-2.0
/*
 * psrace.c - minimal power_supply lifetime-race reproducers for the
 * v7.3 audit report docs/manual-review-drivers-power-2026-11-3.md.
 *
 * action=3 : Case 3, get_property() after power_supply_unregister()/kfree(drvdata)
 * action=4 : Case 4, get_system_batteries() resurrects an unregistered battery
 * action=5 : Case 5B, power_supply_changed() after unregister frees LED triggers
 *
 * The module only uses EXPORT_SYMBOL_GPL power_supply APIs; the test psy is a
 * normal class device whose private data is deliberately freed after unregister.
 */
#define pr_fmt(fmt) "PSRACE: " fmt

#include <linux/atomic.h>
#include <linux/completion.h>
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/err.h>
#include <linux/kthread.h>
#include <linux/module.h>
#include <linux/power_supply.h>
#include <linux/slab.h>
#include <linux/wait.h>

static int action = 3;
module_param(action, int, 0444);
MODULE_PARM_DESC(action, "3=get_property UAF, 4=system-battery resurrection, 5=changed-after-unregister");

static bool control;
module_param(control, bool, 0444);
MODULE_PARM_DESC(control, "1=negative control: do not free/trigger the racy object");

struct ps_priv {
	int magic;
};

static struct device *root;
static struct power_supply *psy;
static struct ps_priv *priv;
static struct task_struct *thr;
static struct completion entered;
static struct completion finished;
static atomic_t armed = ATOMIC_INIT(0);
static atomic_t scope_calls = ATOMIC_INIT(0);
static struct power_supply **sysbatt;
static int sysbatt_cnt;

static int ps_get_property(struct power_supply *psy,
			   enum power_supply_property psp,
			   union power_supply_propval *val)
{
	struct ps_priv *p = power_supply_get_drvdata(psy);

	switch (psp) {
	case POWER_SUPPLY_PROP_PRESENT:
		if (atomic_read(&armed)) {
			pr_info("case3 get_property entered, drvdata=%px\n", p);
			complete(&entered);
			msleep(500);
			val->intval = READ_ONCE(p->magic);
			pr_info("case3 get_property read %#x after unregister\n",
				val->intval);
			complete(&finished);
			return 0;
		}
		/* Case 4 reads drvdata after the battery has been unregistered. */
		val->intval = (action == 4) ? READ_ONCE(p->magic) : 1;
		return 0;
	case POWER_SUPPLY_PROP_ONLINE:
		val->intval = 1;
		return 0;
	case POWER_SUPPLY_PROP_SCOPE:
		/* The first call is the count pass; the second is populate. */
		if (atomic_inc_return(&scope_calls) == 2) {
			pr_info("case4 populate scope callback entered, drvdata=%px\n", p);
			complete(&entered);
			msleep(500);
		}
		val->intval = POWER_SUPPLY_SCOPE_SYSTEM;
		return 0;
	default:
		return -EINVAL;
	}
}

static const enum power_supply_property props3[] = {
	POWER_SUPPLY_PROP_PRESENT,
};
static const enum power_supply_property props4[] = {
	POWER_SUPPLY_PROP_SCOPE,
	POWER_SUPPLY_PROP_PRESENT,
};
static const enum power_supply_property props5[] = {
	POWER_SUPPLY_PROP_ONLINE,
};

static const struct power_supply_desc desc3 = {
	.name = "psrace3",
	.type = POWER_SUPPLY_TYPE_USB,
	.properties = props3,
	.num_properties = ARRAY_SIZE(props3),
	.get_property = ps_get_property,
};
static const struct power_supply_desc desc4 = {
	.name = "psrace4",
	.type = POWER_SUPPLY_TYPE_BATTERY,
	.properties = props4,
	.num_properties = ARRAY_SIZE(props4),
	.get_property = ps_get_property,
};
static const struct power_supply_desc desc5 = {
	.name = "psrace5",
	.type = POWER_SUPPLY_TYPE_USB,
	.properties = props5,
	.num_properties = ARRAY_SIZE(props5),
	.get_property = ps_get_property,
};

static int consumer3(void *unused)
{
	union power_supply_propval val;
	struct power_supply *p;

	msleep(100);
	p = power_supply_get_by_name("psrace3");
	if (!p) {
		pr_err("case3 get_by_name failed\n");
		complete(&finished);
		return 0;
	}
	pr_info("case3 consumer got %px\n", p);
	atomic_set(&armed, 1);
	smp_mb();
	power_supply_get_property(p, POWER_SUPPLY_PROP_PRESENT, &val);
	power_supply_put(p);
	pr_info("case3 consumer put, done\n");
	return 0;
}

static int sysbatt_thread(void *unused)
{
	sysbatt_cnt = power_supply_get_system_batteries(root, &sysbatt);
	pr_info("case4 get_system_batteries -> %d\n", sysbatt_cnt);
	complete(&finished);
	return 0;
}

static int run_case3(void)
{
	int ret;

	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
	if (!priv)
		return -ENOMEM;
	priv->magic = 0x12345678;

	psy = power_supply_register(root, &desc3, &(struct power_supply_config){
		.drv_data = priv,
		.no_wakeup_source = true,
	});
	if (IS_ERR(psy)) {
		ret = PTR_ERR(psy);
		pr_err("case3 register failed %d\n", ret);
		kfree(priv);
		return ret;
	}

	thr = kthread_run(consumer3, NULL, "psrace3c");
	if (IS_ERR(thr))
		return PTR_ERR(thr);

	ret = wait_for_completion_timeout(&entered, 5 * HZ);
	if (!ret) {
		pr_err("case3 callback did not enter\n");
		return -ETIMEDOUT;
	}

	if (control) {
		wait_for_completion_timeout(&finished, 5 * HZ);
		pr_info("case3 control: callback completed before free\n");
		msleep(500);
		power_supply_unregister(psy);
		kfree(priv);
		priv = NULL;
		pr_info("case3 control done (no KASAN expected)\n");
		return 0;
	}

	pr_info("case3 unregister + kfree(drvdata)\n");
	power_supply_unregister(psy);
	kfree(priv);
	priv = NULL;

	/* callback wakes up and reads freed drvdata */
	msleep(1000);
	pr_info("case3 init done\n");
	return 0;
}

static int run_case4(void)
{
	int ret;
	union power_supply_propval val;

	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
	if (!priv)
		return -ENOMEM;
	priv->magic = 0x4b4b4b4b;

	psy = power_supply_register(root, &desc4, &(struct power_supply_config){
		.drv_data = priv,
		.no_wakeup_source = true,
	});
	if (IS_ERR(psy)) {
		ret = PTR_ERR(psy);
		pr_err("case4 register failed %d\n", ret);
		kfree(priv);
		return ret;
	}

	thr = kthread_run(sysbatt_thread, NULL, "psrace4s");
	if (IS_ERR(thr))
		return PTR_ERR(thr);

	ret = wait_for_completion_timeout(&entered, 5 * HZ);
	if (!ret) {
		pr_err("case4 populate callback did not enter\n");
		return -ETIMEDOUT;
	}

	if (control) {
		wait_for_completion_timeout(&finished, 5 * HZ);
		if (sysbatt_cnt > 0) {
			pr_info("case4 control: read array[0]=%px before free\n",
				sysbatt[0]);
			power_supply_get_property(sysbatt[0],
						  POWER_SUPPLY_PROP_PRESENT,
						  &val);
			power_supply_put_system_batteries(sysbatt, sysbatt_cnt);
		}
		power_supply_unregister(psy);
		kfree(priv);
		priv = NULL;
		pr_info("case4 control done (no KASAN expected)\n");
		return 0;
	}

	pr_info("case4 unregister + kfree(drvdata) during populate\n");
	power_supply_unregister(psy);
	kfree(priv);
	priv = NULL;

	ret = wait_for_completion_timeout(&finished, 5 * HZ);
	if (!ret) {
		pr_err("case4 get_system_batteries did not finish\n");
		return -ETIMEDOUT;
	}

	if (sysbatt_cnt <= 0) {
		pr_err("case4 no battery resurrected (%d)\n", sysbatt_cnt);
		return -ENOENT;
	}

	pr_info("case4 array[0]=%px (unregistered battery), now get_property\n",
		sysbatt[0]);
	power_supply_get_property(sysbatt[0], POWER_SUPPLY_PROP_PRESENT, &val);
	power_supply_put_system_batteries(sysbatt, sysbatt_cnt);
	pr_info("case4 init done\n");
	return 0;
}

static int run_case5(void)
{
	int ret;

	priv = kzalloc(sizeof(*priv), GFP_KERNEL);
	if (!priv)
		return -ENOMEM;
	priv->magic = 0x5a5a5a5a;

	psy = power_supply_register(root, &desc5, &(struct power_supply_config){
		.drv_data = priv,
		.no_wakeup_source = true,
	});
	if (IS_ERR(psy)) {
		ret = PTR_ERR(psy);
		pr_err("case5 register failed %d\n", ret);
		kfree(priv);
		return ret;
	}

	pr_info("case5 online_trig=%px\n", psy->online_trig);
	if (!psy->online_trig) {
		pr_err("case5 no online trigger, abort\n");
		return -ENOENT;
	}

	/*
	 * Emulate a consumer that keeps both the struct power_supply and a
	 * positive use_cnt alive.  Without the use_cnt, get_property() in
	 * changed_work would return -ENODEV before reaching the trigger.
	 */
	{
		struct power_supply *held = power_supply_get_by_name("psrace5");

		if (!held) {
			pr_err("case5 get_by_name failed\n");
			return -ENOENT;
		}
		pr_info("case5 held=%px use_cnt now >0\n", held);
	}
	pr_info("case5 unregister (frees LED trigger), then changed\n");
	power_supply_unregister(psy);
	pr_info("case5 after unregister online_trig=%px\n", psy->online_trig);
	if (control) {
		msleep(1000);
		power_supply_put(psy);
		pr_info("case5 control done (no changed, no KASAN expected)\n");
		return 0;
	}
	power_supply_changed(psy);
	msleep(1000);
	/* Drop the consumer reference only after changed_work has run. */
	power_supply_put(psy);
	pr_info("case5 init done\n");
	return 0;
}

static int __init psrace_init(void)
{
	int ret;

	root = root_device_register("psrace-root");
	if (IS_ERR(root))
		return PTR_ERR(root);
	init_completion(&entered);
	init_completion(&finished);

	switch (action) {
	case 3:
		ret = run_case3();
		break;
	case 4:
		ret = run_case4();
		break;
	case 5:
		ret = run_case5();
		break;
	default:
		pr_err("unknown action %d\n", action);
		ret = -EINVAL;
	}
	pr_info("action=%d init ret=%d\n", action, ret);
	return ret;
}

module_init(psrace_init);
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("power_supply v7.3 lifetime-race reproducer");