[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?

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

[Carlo Szelinsky]

On Wed, 24 Jun 2026 16:12:51 +0100, Simon Horman wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
[...]
> [High]
> Does this fix the use-after-free completely, or only the regulator-put
> part of it?
[...]
> 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?

Thanks, the review is correct. This patch only fixes the regulator
handle. In the same unbind-while-held case __pse_control_release()
also reads psec->pcdev->pi[] and psec->pcdev->owner after
pse_controller_unregister() has freed pcdev->pi, so those are still
use-after-free reads on their own.

That wider problem is exactly what you describe: the controller does
not drain its outstanding pse_control references on unregister. It is
fixed by draining them, which is what the PSE notifier series does --
PSE_UNREGISTERED drops every phydev->psec before pse_release_pis()
frees pcdev->pi. This patch is patch 1 of that series (by Corey
Leavitt); the rest targets net-next and is deferred until it reopens:

  https://lore.kernel.org/netdev/20260620112440.1734404-1-github@szelinsky.de/

Jakub suggested sending this one to net on its own since it is a fix,
so it is here without the notifier patches. My v1 commit message
overclaimed by saying it makes __pse_control_release() correct
regardless of the controller's devres state, which is only true for
the regulator handle. I have reworded it in v2 to scope it to the
regulator put and to point at the series for the wider lifetime fix.

Does you agree? Another option would be to wait for the entire series.

cheers Carlo