Re: [PATCH net v2 10/14] net: pse-pd: keep the PI data until the PI regulators are gone

[Oleksij Rempel <o.rempel@pengutronix.de>]

On Tue, Oct 20, 2026 at 07:12:45PM +0200, Carlo Szelinsky wrote:
> pse_controller_unregister() frees the PI array, but the PI regulators
> are only unregistered after it, by devres. Until then their ops still
> run, for example on a read of the regulator's sysfs "state" or when a
> consumer enables or disables the PI, and they read the freed array.
> 
> Free the PI array from a devm action that is added before the PI
> regulators, so devres frees it after them. This also frees it when
> registration fails, where it was leaked before.
> 
> While the controller goes away, the PSE core must not power a PI.
> pse_controller_unregister() now first sets pcdev->unregistering under
> pcdev->lock. After that, enabling a PI fails with -ENODEV, and so does
> _pse_pi_delivery_power_sw_pw_ctrl(), which pse_isr(), a priority change
> and the retry after a disable use to power a PI. Disabling a PI still
> switches it off and frees its budget.
> 
> pse_flush_pw_ds() also switches off each PI that holds budget and gives
> the budget back before it drops the power domain. Nothing controls such
> a PI once its controller is gone, and before, its budget stayed
> reserved on the supply. If a PI cannot be switched off, its budget
> stays reserved, as the PI may still draw it.
> 
> Triggered in QEMU (arm64, KASAN) with a test-only PSE controller:
> reading the PI regulators' sysfs "state" while the controller unbinds
> gives
> 
>   BUG: KASAN: slab-use-after-free in pse_pi_is_enabled+0x294/0x2d4
> 
> and unbinding it with budget reserved leaves that budget on the
> supply. With this patch both are gone, the PIs are off after the
> unbind, and a priority change during the unbind no longer powers a PI.
> pse_isr() is not covered by the test, as the test controller has no
> IRQ.
> 
> Fixes: d83e13761d5b ("net: pse-pd: Use regulator framework within PSE framework")
> Fixes: ffef61d6d273 ("net: pse-pd: Add support for budget evaluation strategies")
> Signed-off-by: Carlo Szelinsky <github@szelinsky.de>

Freeing the PI array from devres after the PI regulators, and refusing
to power a PI once unregistering has started, look good to me.

Please drop pse_pi_power_off() from this patch, and the text about it
in the commit message and the cover letter.

The PI regulators follow the regulator model. A regulator that goes
away does not switch its output off; the consumer has to disable it
before it calls regulator_put(). For a PI the consumer is the PSE core
itself, on pcdev->dev. So on unbind every enabled PI should go off
through the normal disable path, before pse_flush_pw_ds() clears
pi->pw_d. The disable path also gives the budget back.

pse_pi_power_off() switches off only the PIs that hold budget. That is
not enough: a PI under hardware control, or under software control
without pi_get_pw_req(), can deliver power with no budget allocated.

Switching the PIs off on unbind belongs to the fix for a pse_control
outliving its controller, not to this patch. Patch 11 calls
pse_pi_power_off() from pse_pi_take_pw_d(), so patch 11 needs to drop
that call, and the comment above it, too.

Here "off" means what IEEE 802.3 calls mr_pse_enable = disable: no
power and no detection, as if there was no PSE. pi_disable() does not
do this on every driver today. tps23881 turns detection and
classification back on after power off, and the core relies on
detection staying on, since it also uses pi_disable() to power off a
PI that stays admin enabled. Fixing tps23881 is not part of this
series.

pw-bot: cr

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |