power: supply: core: system-battery enumeration can return an unregistered battery

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

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

power_supply_get_system_batteries() counts system batteries in one class
iteration and fills the array in a second one.  The count pass calls
power_supply_is_system_battery(), which uses
power_supply_get_property_direct() without epc_sem or a removing check; a
failed _direct read (including -ENODEV after unregister) is treated as
"not DEVICE" and still returns true.  The fill pass then calls
get_device() and atomic_inc(&psy->use_cnt) on the already unregistered
psy, putting a dead battery into the returned array.  tcpm is the only
in-tree caller; it caches the array for the port lifetime.  KASAN
reproduced the resulting UAF on 7.3.

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 upstream 9f7f7a859451 fix adds the epc_sem/removing check to the
  get/set wrappers, but deliberately leaves
  power_supply_get_property_direct() lockless.  It therefore does not
  cover this enumeration path; 7.3 is affected and mainline still has
  the two-pass logic and the _direct call.

* Mechanism

  power_supply_is_system_battery() (:480-493) does:

      if (psy->desc->type != POWER_SUPPLY_TYPE_BATTERY)
          return false;
      if (!power_supply_get_property_direct(psy,
                                            POWER_SUPPLY_PROP_SCOPE, &val))
          if (val.intval == POWER_SUPPLY_SCOPE_DEVICE)
              return false;
      return true;

  The _direct helper (:1442-1447) calls __power_supply_get_property()
  with lock=false.  The helper only checks use_cnt (:1397-1401) and then
  calls the driver callback; there is no epc_sem and no removing check.
  If the battery is unregistered between the two passes, the direct
  SCOPE read returns -ENODEV; the code treats that as "not DEVICE" and
  returns true, so the battery remains a "system battery" for the count.

  In the fill pass (:527-544):

      if (power_supply_is_system_battery(epsy)) {
          if (data->size <= data->cnt)
              return -EOVERFLOW;
          get_device(&epsy->dev);
          data->psys[data->cnt] = epsy;
          atomic_inc(&epsy->use_cnt);
          data->cnt++;
      }

  get_device() only keeps the struct device; it does not keep the
  driver's drv_data.  klist_del() from device_del() does not wait for
  iterator users, so this get_device() can succeed after unregister.
  atomic_inc() then moves use_cnt from 0 back to 1, so a later
  get_property() passes the use_cnt test (and, without the 9f7f guard,
  reaches the freed driver callback).

  Both passes are separate class_for_each_device() walks
  (:495-519 and :527-544); there is no lock held across them and no
  generation counter.  power_supply_get_system_batteries() (:588-611)
  allocates the array after the count, so the window also includes an
  unbounded kzalloc.

* Concrete corruption / impact

  The returned array can contain a battery whose power_supply was
  already unregistered; tcpm then uses it for PD battery status and
  capacity requests.  Without the 9f7f guard, the follow-up
  get_property() reads freed drv_data; KASAN reports
  ps_get_property+0x5d/0x130, Read of size 4 at ff110000034f69a0.
  Even with the 9f7f guard, the array is stale and tcpm will keep
  querying a removed battery, returning -ENODEV or wrong data until the
  port is unregistered.  I have not demonstrated privilege escalation.

* Trigger model

  tcpm is the only external caller (tcpm.c:1510).  It enumerates system
  batteries only when the port is self-powered (tcpm.c:8326) and caches
  them until tcpm_unregister_port() (:8988).  A battery supplier
  unbind/remove (or failed probe) concurrent with
  tcpm_get_fixed_batt() is the trigger; a PD peer can request
  GET_BATTERY_STATUS/BATT_CAP, which reaches
  tcpm_pd_send_batt_status()/batt_cap().  Requires a self-powered
  Type-C/PD port and a removable/async-probed battery; not a generic
  unprivileged userspace interface.

* Reproducer / results

  psrace.c (attached, text/plain), plan=ps4.  It registers a BATTERY
  test psy whose SCOPE callback returns SYSTEM; the count pass is fast,
  the populate pass signals entry and sleeps; the init thread then
  unregisters and frees drv_data.  The API returns cnt=1 with
  array[0] pointing at the unregistered battery; a subsequent
  get_property(PRESENT) reads the freed drvdata.

  Observed on 7.3:

      get_system_batteries -> 1
      array[0]=ff11000003109000 (unregistered battery)
      BUG: KASAN: slab-use-after-free in ps_get_property+0x5d/0x130
      Read of size 4 at ff110000034f69a0

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


* Suggested direction
  Make the enumeration robust against concurrent removal: take
  epc_sem/read and reject psy->removing in is_system_battery(); treat
  -ENODEV/-EAGAIN from the direct SCOPE read as "not a system battery";
  after get_device() in the fill pass, revalidate the psy (or retry the
  whole enumeration) before atomic_inc(); and consider exposing a
  stable snapshot/generation to callers.  tcpm should also invalidate
  fixed_batt on a battery notifier/removal instead of caching forever.
  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");