power: supply: core: get/set property callbacks can run after unregister and read freed driver data

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

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

power_supply_get_property() and power_supply_set_property() can invoke a
driver's ->get_property()/->set_property() callback after
power_supply_unregister() has set psy->removing and after the driver's
private data has been freed.  The wrappers take neither psy->epc_sem nor
check psy->removing; their only check is a check-then-act on
psy->use_cnt, which protects the struct power_supply device reference but
not devm-allocated driver data.  A consumer that obtained the psy before
unregister can therefore read freed drv_data.  I reproduced the
resulting KASAN slab-use-after-free on an unmodified 7.3 kernel with a
minimal out-of-tree consumer; tcpm, sun4i-usb, bq25890 and
sc27xx-fuel-gauge contain in-tree consumers with the same pattern.

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 (generic/outline/stack/vmalloc),
  QEMU/KVM.  CONFIG_POWER_SUPPLY=y, CONFIG_POWER_SUPPLY_HWMON=y,
  CONFIG_LEDS_TRIGGERS=y, CONFIG_MODULES=y.  The kernel was built from
  the same build tree used for the supplied module; no source changes.

  The upstream fix 9f7f7a859451 ("power_supply: serialize property
  access with unregister") adds the missing guard to these two wrappers
  on mainline.  The 7.3 tree I tested does not contain it; this report
  documents the 7.3 exposure and requests a backport.  I have not
  attached a new patch.

* Mechanism

  power_supply_get_property() (power_supply_core.c:1422-1427) is:

      return __power_supply_get_property(psy, psp, val, true);

  and power_supply_set_property() (:1479-1484) is:

      return __power_supply_set_property(psy, psp, val, true);

  Neither wrapper takes psy->epc_sem or tests psy->removing.
  __power_supply_get_property() does:

      if (atomic_read(&psy->use_cnt) <= 0) {
          if (!psy->initialized)
              return -EAGAIN;
          return -ENODEV;
      }

  (:1397-1401) and then calls psy->desc->get_property() (:1414-1415);
  the set helper has the same use_cnt test at :1455 and calls
  psy->desc->set_property() at :1473-1476.

  use_cnt is not a lifetime barrier for the driver's private data.  A
  consumer gets a device reference and increments use_cnt in
  power_supply_get_by_name() (:465-478) or
  power_supply_get_by_reference() (:664-687); unregister drops the
  registration's own reference at :1930, but the consumer's reference
  keeps the struct power_supply and use_cnt alive.  The driver's
  drv_data is normally devm_kzalloc'ed on the parent device and is freed
  when the parent's remove path finishes, independently of the struct
  power_supply.  unregister does set removing under epc_sem write
  (:1939-1940), cancels work (:1942-1943) and device_unregisters
  (:1949), but none of that is observed by the get/set wrappers.

  The interleaving:

    A: consumer calls get_property(); __... reads use_cnt == 1 and
       passes the test.
    B: supplier's remove path calls power_supply_unregister(); it sets
       removing, cancels work, device_unregisters, and the parent
       remove frees drv_data (or devres releases it).
    C: consumer enters desc->get_property() and dereferences drv_data.

  A second variant uses psy->battery_info: it is devm_kzalloc'ed on
  psy->dev (:768), installed at :1783-1787, and released by
  devres_release_all() from device_del() (drivers/base/core.c:3980)
  before kobject_del().  A consumer that already holds the psy can reach
  the battery_info fallback at :1416-1417 after that release; tcpm's
  CHARGE_FULL_DESIGN/CHARGE_FULL reads (tcpm.c:1681-1697) are a
  concrete caller.  This is still the same wrapper/lifetime bug, not a
  separate explicit power_supply_put_battery_info() race.

  sysfs is not an independent trigger here: device_remove_attrs()
  (drivers/base/core.c:3964) drains the attribute files before
  devres_release_all() at :3980.  The reachable path is a cross-device
  caller that already cached the psy.

* Concrete corruption / impact

  KASAN reports a slab-use-after-free read in the driver's
  ->get_property() callback, e.g. ps_get_property+0x7c/0x110, Read of
  size 4 at ff11000003721a80; the alloc and free stacks are both in the
  test consumer/driver path.  With a freed object reallocated, the
  callback can return stale or reused memory as a property value; with
  poisoned memory it can crash.  unregister also hits
  WARN_ON(atomic_dec_return(&psy->use_cnt)) at :1930 because the
  consumer's reference is still held; with panic_on_warn this is a
  local DoS.  I have not demonstrated privilege escalation.

* Trigger model

  A consumer driver must cache the supplier psy across the supplier's
  unbind/remove.  In-tree examples:

    - tcpm caches the system batteries in port->fixed_batt
      (tcpm.c:1503-1515), uses them for PD battery status/capacity
      (:1607-1638, :1681-1697), and only puts them in
      tcpm_unregister_port() (:8988).
    - sun4i-usb caches the vbus supply (phy-sun4i-usb.c:757) and reads
      PRESENT from a polling work (:427, :596).
    - bq25890 caches a secondary charger (bq25890_charger.c:1409) and
      calls set_property(ONLINE) from its pump work (:1066-1068,
      :1102-1104).
    - sc27xx-fuel-gauge has a short get_by_name()/get_property() window
      (sc27xx_fuel_gauge.c:556-566).

  The trigger is a supplier unbind, failed probe/remove, or module
  unload while the consumer stays alive; for tcpm a PD peer can request
  battery status.  The test module is loaded by root, but the real paths
  are hardware/remove driven, not a generic unprivileged userspace
  interface.  No special hardware is required for the core reproduction
  beyond the module.

* Reproducer / results

  Out-of-tree module psrace.c (attached, text/plain), built with:

      make -C <build-kasan-7.3> CC=clang M=$PWD modules

  and run under the supplied QEMU image with plan=ps3.  The module
  registers a test psy, has its get_property() signal entry, sleep, and
  read drv_data; a consumer thread calls get_by_name() then
  get_property(); the init thread unregisters and kfree()s drv_data.

  Observed on 7.3:

      BUG: KASAN: slab-use-after-free in ps_get_property+0x7c/0x110
      Read of size 4 at addr ff11000003721a80
      ...
      WARNING: ... power_supply_core.c:1930 ...

  A control run (same module, control=1) waits for the callback to
  finish before unregister/free and reports no KASAN.  Full logs are
  available on request.


* Suggested direction

  Apply the 9f7f7a859451 guard to both wrappers:

      guard(rwsem_read)(&psy->epc_sem);
      if (psy->removing)
          return -ENODEV;

  and keep the _direct variants lockless for extension callbacks.  A
  stable backport to 7.3 is the immediate request.  I have not attached
  a patch; happy to test one.

  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");