[PATCH net] net: pse-pd: scope pse_control regulator handle to kref lifetime

[PATCH net] net: pse-pd: scope pse_control regulator handle to kref lifetime

[Carlo Szelinsky]

From: Corey Leavitt <corey@leavitt.info>

__pse_control_release() drops psec->ps via devm_regulator_put(), which
only succeeds if the devres entry added by the matching
devm_regulator_get_exclusive() is still present on pcdev->dev at the
time the pse_control's kref hits zero.

In practice that assumption does not hold when the controller is
unbound while any pse_control still has consumers: pcdev->dev's
devres list is released LIFO, so every per-attach regulator-GET
devres runs (and regulator_put()s the underlying regulator) before
pse_controller_unregister() itself is invoked. Any later
pse_control_put() from that unbind path then reads psec->ps as a
dangling pointer inside devm_regulator_put() and WARNs at
drivers/regulator/devres.c:232 (devres_release() fails to find the
already-released match).

The pse_control's consumer handle is logically scoped to the
pse_control's refcount, not to pcdev->dev's devres lifetime. Switch
to the plain regulator_get_exclusive() / regulator_put() pair so
__pse_control_release() does the right put regardless of whether
the controller's devres has already been unwound.

No change to the regulator-framework-visible refcount or lifetime of
the underlying regulator: a single get paired with a single put. The
existing devm_regulator_register() for the per-PI rails is unchanged
(those ARE correctly scoped to the controller's lifetime).

Fixes: d83e13761d5b ("net: pse-pd: Use regulator framework within PSE framework")
Signed-off-by: Corey Leavitt <corey@leavitt.info>
Acked-by: Kory Maincent <kory.maincent@bootlin.com>
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
---
This was patch 1 of the "decouple controller lookup from MDIO probe"
series [1]. Reposting it standalone for net as suggested, since it is a
self-contained fix. The rest of the series targets net-next and will be
resent once net-next reopens.

[1] https://lore.kernel.org/netdev/20260620112440.1734404-1-github@szelinsky.de/
---
 drivers/net/pse-pd/pse_core.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
index 69dbdbde9d71..a5e6d7b26b9f 100644
--- a/drivers/net/pse-pd/pse_core.c
+++ b/drivers/net/pse-pd/pse_core.c
@@ -1367,7 +1367,7 @@ static void __pse_control_release(struct kref *kref)
 
 	if (psec->pcdev->pi[psec->id].admin_state_enabled)
 		regulator_disable(psec->ps);
-	devm_regulator_put(psec->ps);
+	regulator_put(psec->ps);
 
 	module_put(psec->pcdev->owner);
 
@@ -1436,8 +1436,8 @@ pse_control_get_internal(struct pse_controller_dev *pcdev, unsigned int index,
 		goto free_psec;
 
 	pcdev->pi[index].admin_state_enabled = ret;
-	psec->ps = devm_regulator_get_exclusive(pcdev->dev,
-						rdev_get_name(pcdev->pi[index].rdev));
+	psec->ps = regulator_get_exclusive(pcdev->dev,
+					   rdev_get_name(pcdev->pi[index].rdev));
 	if (IS_ERR(psec->ps)) {
 		ret = PTR_ERR(psec->ps);
 		goto put_module;

base-commit: d07d80b6a129a44538cda1549b7acf95154fb197
-- 
2.43.0

Re: [PATCH net] net: pse-pd: scope pse_control regulator handle to kref lifetime

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://netdev-ai.bots.linux.dev/sashiko/
---
net: pse-pd: scope pse_control regulator handle to kref lifetime

This patch switches the pse_control consumer regulator handle from
devm_regulator_get_exclusive() / devm_regulator_put() to the plain
regulator_get_exclusive() / regulator_put() pair, so the put in
__pse_control_release() works regardless of whether the controller's
devres has already been unwound.

> diff --git a/drivers/net/pse-pd/pse_core.c b/drivers/net/pse-pd/pse_core.c
> index 69dbdbde9d71..a5e6d7b26b9f 100644
> --- a/drivers/net/pse-pd/pse_core.c
> +++ b/drivers/net/pse-pd/pse_core.c
> @@ -1367,7 +1367,7 @@ static void __pse_control_release(struct kref *kref)
>  
>  	if (psec->pcdev->pi[psec->id].admin_state_enabled)
>  		regulator_disable(psec->ps);
> -	devm_regulator_put(psec->ps);
> +	regulator_put(psec->ps);
>  
>  	module_put(psec->pcdev->owner);

[High]
Does this fix the use-after-free completely, or only the regulator-put
part of it?

The commit message describes the scenario where the controller is
unbound while a pse_control still has consumers, and explains that
pcdev->dev's devres is released LIFO before pse_controller_unregister()
runs.

In that same scenario, __pse_control_release() still reads
psec->pcdev->pi[psec->id].admin_state_enabled and psec->pcdev->owner
just before the regulator_put():

	if (psec->pcdev->pi[psec->id].admin_state_enabled)
		regulator_disable(psec->ps);
	regulator_put(psec->ps);

	module_put(psec->pcdev->owner);

pse_controller_unregister() calls pse_release_pis() which does
kfree(pcdev->pi), and pcdev->pi is plain kzalloc memory rather than
devres-managed. pcdev itself is typically a driver-owned allocation
tied to pcdev->dev's lifetime.

If a later pse_control_put() runs from the unbind path after
pse_controller_unregister() has freed pcdev->pi (and pcdev), would
these pcdev->pi[...] and pcdev->owner reads operate on already-freed
memory?

Similarly, would the regulator_put() then be called against an rdev
that has already been torn down through the controller's devres?

This pre-dates this patch, so it is not a regression introduced here,
but the commit message frames the change as making
__pse_control_release() correct "regardless of whether the
controller's devres has already been unwound", which seems to only
cover the regulator handle and not the pcdev->pi / pcdev->owner reads
on the same code path.

Would a more complete fix also need pse_controller_unregister() to
drain outstanding pse_control references, or have pse_control hold a
refcount on pcdev, so that psec cannot outlive pcdev->pi and pcdev?