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

Carlo Szelinsky <[email protected]>
Newsgroups org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <[email protected]>
On Mon, 10 Aug 2026 13:52:33 -0700 Jakub Kicinski wrote:
> It'd be great if we can figure out the safe lifetime rules for pse-pd
> from all entry points. Specifically the sysfs complaint on patch 1
> (assuming it's real) makes the series seem like a bit of of a see-saw.
>
> Let's get it reposted and take it from there?

Thanks for the reviews, Kory.

Makes sense. Let me go through everything that still touches pcdev->pi
once pse_controller_unregister() has started freeing it, so it's clear
what the series covers and what it doesn't.

The regulator ops (pse_pi_enable/disable/is_enabled) are the sysfs case.
It's real: the PI regulators get registered before the controller, so on
unbind the controller goes down first and pse_release_pis() frees
pcdev->pi while the regulators are still live. is_enabled() is
reachable over sysfs. Patch 2 is what handles this - it checks !pcdev->pi
in those three ops and frees under the same lock. Patch 1 just does the
reorder, so the two really need to be read together.

The other one is of_pse_match_pi() from of_pse_control_get(). A consumer
probing at the same time walks pse_controller_list and reads
pcdev->pi[i].np. Problem is we free pi before list_del(), so the
controller is still on the list with pi already gone. The series doesn't
touch this yet. It's a one-liner really - move list_del() before the
free. Then a lookup either sees a valid pi or doesn't find the controller
at all, no NULL checks anywhere. I'd add that as a patch 3.

Last two are pse_ethtool_get_status() and __pse_control_release(), from a
consumer still holding a pse_control. They read pcdev->pi[psec->id] too,
but here the real issue isn't pi, it's that the consumer can outlive the
whole controller - of_pse_control_get() only takes a module ref, no
device link - so a NULL check buys you nothing. That's pre-existing and
not really something to fix in net. The notifier series I have for
net-next [1] is what deals with it: the phy layer drops phydev->psec on
unregister before pi is freed, and phy is the only pse_control user we
have in tree.

So for v3: patches 1 and 2 as they are with Kory's Reviewed-by, plus the
small patch 3 for the list_del reorder, and I'll write all this up in the
cover letter so the ethtool side is clearly left to [1].

Sound ok before I send it out?

[1] https://lore.kernel.org/netdev/[email protected]/

Thanks,
Carlo
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.