[PATCH net v2 0/2] net: pse-pd: fix use-after-free of PI array on controller unregister

[PATCH net v2 0/2] net: pse-pd: fix use-after-free of PI array on controller unregister

[Carlo Szelinsky]

This series fixes use-after-free bugs in the PSE core teardown path.

When a PSE controller is unregistered, pse_controller_unregister() frees
the PI array (pcdev->pi) and the power domain supplies. But the IRQ
handler, the notification worker, and the regulator disable path can
still run at that point and touch this freed memory.

Patch 1 reorders pse_controller_unregister() so the IRQ and the worker
are stopped before anything they use is freed.

Patch 2 clears pcdev->pi under the lock after freeing it, and makes the
three regulator ops that read pcdev->pi return early when it is NULL.

This series closes the use-after-free reachable from the regulator core
and the controller's own IRQ and worker during unregister. The wider
case - a pse_control consumer (the ethtool path) whose handle outlives
the controller - is a separate lifetime problem handled by the PSE
notifier decoupling work for net-next, not this fix.

Both bugs are pre-existing. They are teardown races, so there is no easy
way to trigger them on purpose and no simple reproducer. The fix is based
on code review. It is compile tested and checkpatch clean.

v1: https://lore.kernel.org/all/20260524223306.2570676-1-github@szelinsky.de/

Changes in v2:
- Patch 1: also stop the IRQ before pse_flush_pw_ds(), and cancel the
  notification worker before pse_release_pis(). v1 only moved
  disable_irq() ahead of pse_release_pis(). Also fix the commit message,
  which wrongly said the worker does not touch pcdev->pi.
- Patch 2: take pcdev->lock around the kfree() and the pcdev->pi = NULL
  store, so a reader sees an authoritative NULL. Add the same NULL guard
  to pse_pi_enable() and pse_pi_is_enabled(), not just pse_pi_disable().
- Thanks to Simon Horman for the review.

Carlo Szelinsky (2):
  net: pse-pd: stop async event sources before freeing PI data in
    unregister
  net: pse-pd: guard regulator ops against freed PI data during
    unregister

 drivers/net/pse-pd/pse_core.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

-- 
2.43.0

[PATCH net v2 1/2] net: pse-pd: stop async event sources before freeing PI data in unregister

[Carlo Szelinsky]

pse_controller_unregister() frees resources that its own asynchronous
event sources are still using:

 * The PI array (pcdev->pi) is freed by pse_release_pis() while the
   threaded IRQ handler pse_isr() can still be running. pse_isr() walks
   pcdev->pi[] (via pse_set_config_isr() and
   regulator_notifier_call_chain() on pcdev->pi[i].rdev), so an interrupt
   arriving before disable_irq() dereferences freed memory.

 * pse_flush_pw_ds() runs before disable_irq() and drops the power domain
   references, which can free pw_d->supply via __pse_pw_d_release().
   A concurrent interrupt reaches that supply through
   _pse_pi_disable() -> pse_pw_d_retry_power_delivery() ->
   regulator_request_power_budget(pw_d->supply), another use-after-free.

 * cancel_work_sync(&pcdev->ntf_work) runs after pse_release_pis(), but
   the notification worker reaches pcdev->pi too: pse_send_ntf_worker()
   -> pse_control_put() -> __pse_control_release() dereferences
   psec->pcdev->pi[psec->id].admin_state_enabled. Draining the worker
   after the PI array is freed is therefore also a use-after-free.

Reorder teardown so every asynchronous accessor is stopped first:
disable the IRQ, drain the notification worker, and only then flush the
power domains and release the PI array.

Fixes: ffef61d6d273 ("net: pse-pd: Add support for budget evaluation strategies")
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
 drivers/net/pse-pd/pse_core.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index a5e6d7b26b9f..6045b6c399c2 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -1114,11 +1114,15 @@ EXPORT_SYMBOL_GPL(pse_controller_register);
  */
 void pse_controller_unregister(struct pse_controller_dev *pcdev)
 {
-	pse_flush_pw_ds(pcdev);
-	pse_release_pis(pcdev);
+	/* Stop the IRQ and notification worker before freeing what they
+	 * reach: both touch pcdev->pi, and the IRQ also uses pw_d->supply
+	 * that pse_flush_pw_ds() drops.
+	 */
 	if (pcdev->irq)
 		disable_irq(pcdev->irq);
 	cancel_work_sync(&pcdev->ntf_work);
+	pse_flush_pw_ds(pcdev);
+	pse_release_pis(pcdev);
 	kfifo_free(&pcdev->ntf_fifo);
 	mutex_lock(&pse_list_mutex);
 	list_del(&pcdev->list);
-- 
2.43.0

[PATCH net v2 2/2] net: pse-pd: guard regulator ops against freed PI data during unregister

[Carlo Szelinsky]

The PSE PI regulators are devm-registered inside pse_controller_register(),
which runs before devres_add() arms the controller's own release in
devm_pse_controller_register(). On driver detach devres unwinds in LIFO
order, so pse_controller_unregister() runs first and frees pcdev->pi via
pse_release_pis(); the regulators are torn down afterwards.

When regulator_unregister() flushes a pending disable, the regulator core
invokes pse_pi_disable(), which dereferences pcdev->pi[id] (directly and
via _pse_pi_disable() -> pse_pi_deallocate_pw_budget()). At that point the
PI array is already freed, so this is a use-after-free. pse_pi_enable()
and pse_pi_is_enabled() dereference pcdev->pi[id] the same way and are
reachable by any regulator consumer that keeps a handle across the
teardown window.

Clear pcdev->pi after freeing it and bail out of the three regulator ops
that dereference it when it is NULL. Perform the kfree() and NULL store in
pse_release_pis() under pcdev->lock, and read pcdev->pi under the same lock
in the ops, so the NULL an op observes is authoritative even when the free
runs concurrently on another CPU: the op either sees the live array or
returns without touching freed memory.

The other three regulator ops (pse_pi_get_voltage(),
pse_pi_get_current_limit(), pse_pi_set_current_limit()) do not dereference
pcdev->pi and need no guard.

Fixes: ffef61d6d273 ("net: pse-pd: Add support for budget evaluation strategies")
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
 drivers/net/pse-pd/pse_core.c | 27 ++++++++++++++++++++++++---
 1 file changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index 6045b6c399c2..21ccb5146616 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -144,7 +144,13 @@ static void pse_release_pis(struct pse_controller_dev *pcdev)
 		of_node_put(pcdev->pi[i].pairset[1].np);
 		of_node_put(pcdev->pi[i].np);
 	}
+	/* Free under the lock so the NULL store is authoritative against
+	 * the regulator ops that read pcdev->pi under pcdev->lock.
+	 */
+	mutex_lock(&pcdev->lock);
 	kfree(pcdev->pi);
+	pcdev->pi = NULL;
+	mutex_unlock(&pcdev->lock);
 }
 
 /**
@@ -421,6 +427,11 @@ static int pse_pi_is_enabled(struct regulator_dev *rdev)
 
 	id = rdev_get_id(rdev);
 	mutex_lock(&pcdev->lock);
+	/* Controller may be unregistered (pcdev->pi freed) mid-teardown. */
+	if (!pcdev->pi) {
+		ret = -ENODEV;
+		goto out;
+	}
 	if (pse_pw_d_is_sw_pw_control(pcdev, pcdev->pi[id].pw_d)) {
 		ret = pcdev->pi[id].admin_state_enabled;
 		goto out;
@@ -674,6 +685,11 @@ static int pse_pi_enable(struct regulator_dev *rdev)
 
 	id = rdev_get_id(rdev);
 	mutex_lock(&pcdev->lock);
+	/* Controller may be unregistered (pcdev->pi freed) mid-teardown. */
+	if (!pcdev->pi) {
+		mutex_unlock(&pcdev->lock);
+		return -ENODEV;
+	}
 	if (pse_pw_d_is_sw_pw_control(pcdev, pcdev->pi[id].pw_d)) {
 		/* Manage enabled status by software.
 		 * Real enable process will happen if a port is connected.
@@ -702,15 +718,20 @@ static int pse_pi_enable(struct regulator_dev *rdev)
 static int pse_pi_disable(struct regulator_dev *rdev)
 {
 	struct pse_controller_dev *pcdev = rdev_get_drvdata(rdev);
-	struct pse_pi *pi;
 	int id, ret;
 
 	id = rdev_get_id(rdev);
-	pi = &pcdev->pi[id];
 	mutex_lock(&pcdev->lock);
+	/* Reached via the regulator core's deferred-disable flush after
+	 * pcdev->pi is freed on unregister.
+	 */
+	if (!pcdev->pi) {
+		mutex_unlock(&pcdev->lock);
+		return 0;
+	}
 	ret = _pse_pi_disable(pcdev, id);
 	if (!ret)
-		pi->admin_state_enabled = 0;
+		pcdev->pi[id].admin_state_enabled = 0;
 
 	mutex_unlock(&pcdev->lock);
 	return 0;
-- 
2.43.0