All of lore.kernel.org
 help / color / mirror / Atom feed
From: Carlo Szelinsky <github@szelinsky.de>
To: Oleksij Rempel <o.rempel@pengutronix.de>,
	Kory Maincent <kory.maincent@bootlin.com>,
	Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Cc: Simon Horman <horms@kernel.org>,
	Jonas Jelonek <jelonek.jonas@gmail.com>,
	netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	Carlo Szelinsky <github@szelinsky.de>
Subject: [PATCH net v3 0/3] net: pse-pd: fix use-after-free of PI array on controller unregister
Date: Thu, 13 Aug 2026 23:06:50 +0300	[thread overview]
Message-ID: <20260813200653.980170-1-github@szelinsky.de> (raw)

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 while other paths
can still reach them.

To make it clear what the series covers, here is every path that
dereferences pcdev->pi and how each one is made safe:

- The regulator ops pse_pi_enable/disable/is_enabled(). Reached from the
  regulator core and from regulator sysfs (is_enabled() via the "state"
  attribute). The PI regulators are registered before the controller, so
  on unbind the controller is torn down first and pse_release_pis() frees
  pcdev->pi while the regulators are still live. Patch 2 checks
  !pcdev->pi in the three ops and does the free under pcdev->lock.

- 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. Today
  the PI array is freed before list_del(), so the controller is still on
  the list with pi gone. Patch 3 moves list_del() ahead of the free;
  both run under pse_list_mutex, so a lookup sees either a live pi or no
  controller at all, with no NULL checks on the lookup path.

- pse_ethtool_get_status() and __pse_control_release(), from a consumer
  still holding a pse_control. These read pcdev->pi[psec->id] too, but
  the real issue is that the consumer can outlive the whole controller:
  of_pse_control_get() only takes a module reference, no device link, so
  a NULL check would not help. This is pre-existing and not a net fix.
  It is handled in net-next by the PSE controller notifier series [1],
  where the phy layer drops phydev->psec on unregister before pi is
  freed. phy is the only pse_control consumer in tree.

Patch 1 reorders teardown so the IRQ and the notification worker are
stopped before the power domains and the PI array are freed.

All of these are pre-existing teardown races, so there is no easy way to
trigger them on purpose and no simple reproducer. The fixes are based on
code review. They are compile tested and checkpatch clean.

[1] https://lore.kernel.org/netdev/20260630091125.3162481-1-github@szelinsky.de/

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

Changes in v3:
- New patch 3: unlink the controller from pse_controller_list before
  freeing the PI array, closing the of_pse_control_get() /
  of_pse_match_pi() race.
- Cover letter now maps every pcdev->pi entry point, following the review
  discussion with Kory and Jakub.
- Added Kory Maincent's Reviewed-by to patches 1 and 2.
- Rebased on net/main.

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 (3):
  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
  net: pse-pd: unregister from the controller list before freeing PI
    data

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


base-commit: 3aa1dcaa4f6f5ae08936491e08bd456f331f2d40
-- 
2.43.0


             reply	other threads:[~2026-08-13 20:07 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-13 20:06 Carlo Szelinsky [this message]
2026-08-13 20:06 ` [PATCH net v3 1/3] net: pse-pd: stop async event sources before freeing PI data in unregister Carlo Szelinsky
2026-08-13 20:06 ` [PATCH net v3 2/3] net: pse-pd: guard regulator ops against freed PI data during unregister Carlo Szelinsky
2026-08-13 20:06 ` [PATCH net v3 3/3] net: pse-pd: unregister from the controller list before freeing PI data Carlo Szelinsky

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260813200653.980170-1-github@szelinsky.de \
    --to=github@szelinsky.de \
    --cc=andrew+netdev@lunn.ch \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=jelonek.jonas@gmail.com \
    --cc=kory.maincent@bootlin.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=o.rempel@pengutronix.de \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.