* [PATCH net] net: phy: micrel: Fix potential shared PHC use-after-free
@ 2026-11-07 6:54 Guangshuo Li
2026-11-07 14:40 ` Andrew Lunn
2026-11-08 7:15 ` netdev-bot+sashiko
0 siblings, 2 replies; 3+ messages in thread
From: Guangshuo Li @ 2026-11-07 6:54 UTC (permalink / raw)
To: Andrew Lunn, Heiner Kallweit, Russell King, David S. Miller,
Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: netdev, linux-kernel
The shared PHC outlives the first PHY's managed pin array and is not
unregistered before the package data is freed. Subsequent PHC operations
can therefore access released storage.
Keep the pins at package scope and select a remaining PHY for register
access when a member leaves. Unregister the clock before releasing its
storage on the last member's exit. Pair each member's registration with
cleanup that also runs on probe failure, and serialize clock access with
member removal.
This issue was found by our static analysis tool under development.
Validated by source review, x86-64 W=1 object builds and an extracted-code
lifecycle test with mocked services. Not tested on hardware.
Fixes: ece19502834d ("net: phy: micrel: 1588 support for LAN8814 phy")
Signed-off-by: Guangshuo Li <liguangshuo@mail.sdu.edu.cn>
---
Validation: x86-64 Clang W=1 micrel.o passed with allmodconfig,
allyesconfig and PTP disabled. An extracted-code test with mocked
services passed under GCC ASan/UBSan; this is not a kernel reproducer.
No hardware test or full linked-kernel build.
OpenAI Codex assisted with source review, patch implementation and
commit-message drafting. The submitter has reviewed the changes.
drivers/net/phy/micrel.c | 177 ++++++++++++++++++++++++++++++---------
1 file changed, 139 insertions(+), 38 deletions(-)
diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
index d0b1ad21667b..8c48d00b0ccf 100644
--- a/drivers/net/phy/micrel.c
+++ b/drivers/net/phy/micrel.c
@@ -413,6 +413,10 @@ struct kszphy_type {
/* Shared structure between the PHYs of the same package. */
struct lan8814_shared_priv {
struct phy_device *phydev;
+ struct phy_device *members[PHY_MAX_ADDR];
+ bool lock_initialized;
+ bool coma_released;
+ bool clock_stopping;
struct ptp_clock *ptp_clock;
struct ptp_clock_info ptp_clock_info;
struct ptp_pin_desc *pin_config;
@@ -3257,7 +3261,13 @@ static int lan8814_ts_info(struct mii_timestamper *mii_ts, struct kernel_ethtool
SOF_TIMESTAMPING_RX_HARDWARE |
SOF_TIMESTAMPING_RAW_HARDWARE;
+ mutex_lock(&shared->shared_lock);
+ if (!shared->ptp_clock || shared->clock_stopping) {
+ mutex_unlock(&shared->shared_lock);
+ return -ENODEV;
+ }
info->phc_index = ptp_clock_index(shared->ptp_clock);
+ mutex_unlock(&shared->shared_lock);
info->tx_types =
(1 << HWTSTAMP_TX_OFF) |
@@ -3539,11 +3549,12 @@ static int lan8814_ptpci_gettime64(struct ptp_clock_info *ptpci,
{
struct lan8814_shared_priv *shared = container_of(ptpci, struct lan8814_shared_priv,
ptp_clock_info);
- struct phy_device *phydev = shared->phydev;
+ struct phy_device *phydev;
u32 nano_seconds;
time64_t seconds;
mutex_lock(&shared->shared_lock);
+ phydev = shared->phydev;
lan8814_ptp_clock_get(phydev, &seconds, &nano_seconds);
mutex_unlock(&shared->shared_lock);
ts->tv_sec = seconds;
@@ -3557,9 +3568,10 @@ static int lan8814_ptpci_settime64(struct ptp_clock_info *ptpci,
{
struct lan8814_shared_priv *shared = container_of(ptpci, struct lan8814_shared_priv,
ptp_clock_info);
- struct phy_device *phydev = shared->phydev;
+ struct phy_device *phydev;
mutex_lock(&shared->shared_lock);
+ phydev = shared->phydev;
lan8814_ptp_clock_set(phydev, ts->tv_sec, ts->tv_nsec);
mutex_unlock(&shared->shared_lock);
@@ -3739,9 +3751,10 @@ static int lan8814_ptpci_adjtime(struct ptp_clock_info *ptpci, s64 delta)
{
struct lan8814_shared_priv *shared = container_of(ptpci, struct lan8814_shared_priv,
ptp_clock_info);
- struct phy_device *phydev = shared->phydev;
+ struct phy_device *phydev;
mutex_lock(&shared->shared_lock);
+ phydev = shared->phydev;
lan8814_ptp_clock_step(phydev, delta);
mutex_unlock(&shared->shared_lock);
@@ -3752,7 +3765,7 @@ static int lan8814_ptpci_adjfine(struct ptp_clock_info *ptpci, long scaled_ppm)
{
struct lan8814_shared_priv *shared = container_of(ptpci, struct lan8814_shared_priv,
ptp_clock_info);
- struct phy_device *phydev = shared->phydev;
+ struct phy_device *phydev;
u16 kszphy_rate_adj_lo, kszphy_rate_adj_hi;
bool positive = true;
u32 kszphy_rate_adj;
@@ -3772,6 +3785,7 @@ static int lan8814_ptpci_adjfine(struct ptp_clock_info *ptpci, long scaled_ppm)
kszphy_rate_adj_hi |= PTP_CLOCK_RATE_ADJ_DIR_;
mutex_lock(&shared->shared_lock);
+ phydev = shared->phydev;
lanphy_write_page_reg(phydev, LAN8814_PAGE_COMMON_REGS, PTP_CLOCK_RATE_ADJ_HI,
kszphy_rate_adj_hi);
lanphy_write_page_reg(phydev, LAN8814_PAGE_COMMON_REGS, PTP_CLOCK_RATE_ADJ_LO,
@@ -3875,13 +3889,14 @@ static int lan8814_ptp_perout(struct ptp_clock_info *ptpci,
{
struct lan8814_shared_priv *shared = container_of(ptpci, struct lan8814_shared_priv,
ptp_clock_info);
- struct phy_device *phydev = shared->phydev;
+ struct phy_device *phydev;
struct timespec64 ts_on, ts_period;
s64 on_nsec, period_nsec;
int pulse_width;
int pin, event;
mutex_lock(&shared->shared_lock);
+ phydev = shared->phydev;
event = rq->perout.index;
pin = ptp_find_pin(shared->ptp_clock, PTP_PF_PEROUT, event);
if (pin < 0 || pin >= LAN8814_PTP_PEROUT_NUM) {
@@ -4047,7 +4062,7 @@ static int lan8814_ptp_extts(struct ptp_clock_info *ptpci,
{
struct lan8814_shared_priv *shared = container_of(ptpci, struct lan8814_shared_priv,
ptp_clock_info);
- struct phy_device *phydev = shared->phydev;
+ struct phy_device *phydev;
int pin;
pin = ptp_find_pin(shared->ptp_clock, PTP_PF_EXTTS,
@@ -4056,6 +4071,7 @@ static int lan8814_ptp_extts(struct ptp_clock_info *ptpci,
return -EINVAL;
mutex_lock(&shared->shared_lock);
+ phydev = shared->phydev;
if (on)
lan8814_ptp_extts_on(phydev, pin, rq->extts.flags);
else
@@ -4331,7 +4347,10 @@ static int lan8814_handle_gpio_interrupt(struct phy_device *phydev, u16 status)
int ret;
mutex_lock(&shared->shared_lock);
- ret = lan8814_gpio_process_cap(shared);
+ if (shared->ptp_clock && !shared->clock_stopping)
+ ret = lan8814_gpio_process_cap(shared);
+ else
+ ret = -ENODEV;
mutex_unlock(&shared->shared_lock);
return ret;
@@ -4559,20 +4578,15 @@ static void lan8814_ptp_init(struct phy_device *phydev)
phydev->default_timestamp = true;
}
-static int __lan8814_ptp_probe_once(struct phy_device *phydev, char *pin_name,
- int gpios)
+static int lan8814_ptp_register(struct phy_device *phydev, char *pin_name,
+ int gpios)
{
struct lan8814_shared_priv *shared = phy_package_get_priv(phydev);
shared->phydev = phydev;
- /* Initialise shared lock for clock*/
- mutex_init(&shared->shared_lock);
-
- shared->pin_config = devm_kmalloc_array(&phydev->mdio.dev,
- gpios,
- sizeof(*shared->pin_config),
- GFP_KERNEL);
+ shared->pin_config = kmalloc_array(gpios, sizeof(*shared->pin_config),
+ GFP_KERNEL);
if (!shared->pin_config)
return -ENOMEM;
@@ -4612,13 +4626,12 @@ static int __lan8814_ptp_probe_once(struct phy_device *phydev, char *pin_name,
if (IS_ERR(shared->ptp_clock)) {
phydev_err(phydev, "ptp_clock_register failed %pe\n",
shared->ptp_clock);
+ shared->ptp_clock = NULL;
+ kfree(shared->pin_config);
+ shared->pin_config = NULL;
return -EINVAL;
}
- /* Check if PHC support is missing at the configuration level */
- if (!shared->ptp_clock)
- return 0;
-
phydev_dbg(phydev, "successfully registered ptp clock\n");
/* The EP.4 is shared between all the PHYs in the package and also it
@@ -4636,12 +4649,90 @@ static int __lan8814_ptp_probe_once(struct phy_device *phydev, char *pin_name,
return 0;
}
-static int lan8814_ptp_probe_once(struct phy_device *phydev)
+static void lan8814_ptp_leave(void *data)
+{
+ struct phy_device *phydev = data;
+ struct lan8814_shared_priv *shared = phy_package_get_priv(phydev);
+ struct phy_device *next = NULL;
+ unsigned int i;
+
+ phy_package_lock(phydev);
+ mutex_lock(&shared->shared_lock);
+ shared->members[phydev->mdio.addr] = NULL;
+ for (i = 0; i < ARRAY_SIZE(shared->members); i++) {
+ if (shared->members[i]) {
+ next = shared->members[i];
+ break;
+ }
+ }
+ if (next) {
+ shared->phydev = next;
+ mutex_unlock(&shared->shared_lock);
+ phy_package_unlock(phydev);
+ return;
+ }
+
+ shared->clock_stopping = true;
+ mutex_unlock(&shared->shared_lock);
+ /* Unregister calls enable() to disable events, using the last PHY. */
+ ptp_clock_unregister(shared->ptp_clock);
+ mutex_lock(&shared->shared_lock);
+ shared->ptp_clock = NULL;
+ shared->phydev = NULL;
+ kfree(shared->pin_config);
+ shared->pin_config = NULL;
+ shared->clock_stopping = false;
+ mutex_unlock(&shared->shared_lock);
+ phy_package_unlock(phydev);
+}
+
+static int lan8814_ptp_join(struct phy_device *phydev, char *pin_name,
+ int gpios)
+{
+ struct lan8814_shared_priv *shared = phy_package_get_priv(phydev);
+ int ret = 0;
+
+ phy_package_lock(phydev);
+ if (!shared->lock_initialized) {
+ mutex_init(&shared->shared_lock);
+ shared->lock_initialized = true;
+ }
+ if (!IS_ENABLED(CONFIG_PTP_1588_CLOCK)) {
+ phy_package_unlock(phydev);
+ return 0;
+ }
+ mutex_lock(&shared->shared_lock);
+ if (!shared->ptp_clock)
+ ret = lan8814_ptp_register(phydev, pin_name, gpios);
+ if (!ret)
+ shared->members[phydev->mdio.addr] = phydev;
+ mutex_unlock(&shared->shared_lock);
+ phy_package_unlock(phydev);
+ if (ret)
+ return ret;
+
+ return devm_add_action_or_reset(&phydev->mdio.dev, lan8814_ptp_leave,
+ phydev);
+}
+
+static void lan8814_remove(struct phy_device *phydev)
+{
+ struct lan8814_shared_priv *shared;
+
+ if (!phydev->shared)
+ return;
+
+ shared = phy_package_get_priv(phydev);
+ if (shared->members[phydev->mdio.addr])
+ devm_release_action(&phydev->mdio.dev, lan8814_ptp_leave, phydev);
+}
+
+static int lan8814_ptp_probe(struct phy_device *phydev)
{
if (!lan8814_has_ptp(phydev))
return 0;
- return __lan8814_ptp_probe_once(phydev, "lan8814_ptp_pin",
+ return lan8814_ptp_join(phydev, "lan8814_ptp_pin",
LAN8814_PTP_GPIO_NUM);
}
@@ -4747,6 +4838,7 @@ static void lan8814_update_meas_time(struct phy_device *phydev)
static int lan8814_probe(struct phy_device *phydev)
{
const struct kszphy_type *type = phydev->drv->driver_data;
+ struct lan8814_shared_priv *shared;
struct kszphy_priv *priv;
u16 addr;
int err;
@@ -4765,8 +4857,11 @@ static int lan8814_probe(struct phy_device *phydev)
* phy address value
*/
addr = lanphy_read_page_reg(phydev, LAN8814_PAGE_COMMON_REGS, 0) & 0x1F;
- devm_phy_package_join(&phydev->mdio.dev, phydev,
- addr, sizeof(struct lan8814_shared_priv));
+ err = devm_phy_package_join(&phydev->mdio.dev, phydev, addr,
+ sizeof(struct lan8814_shared_priv));
+ if (err)
+ return err;
+ shared = phy_package_get_priv(phydev);
/* There are lan8814 SKUs that don't support PTP. Make sure that for
* those skus no PTP device is created. Here we check if the SKU
@@ -4780,15 +4875,21 @@ static int lan8814_probe(struct phy_device *phydev)
priv->is_ptp_available = err == LAN8814_REV_LAN8814 ||
err == LAN8814_REV_LAN8818;
- if (phy_package_probe_once(phydev)) {
+ phy_package_lock(phydev);
+ if (!shared->coma_released) {
err = lan8814_release_coma_mode(phydev);
- if (err)
- return err;
-
- err = lan8814_ptp_probe_once(phydev);
- if (err)
- return err;
+ if (!err)
+ shared->coma_released = true;
+ } else {
+ err = 0;
}
+ phy_package_unlock(phydev);
+ if (err)
+ return err;
+
+ err = lan8814_ptp_probe(phydev);
+ if (err)
+ return err;
lan8814_ptp_init(phydev);
@@ -6143,9 +6244,9 @@ static int ksz9131_resume(struct phy_device *phydev)
#define LAN8842_PTP_GPIO_NUM 16
-static int lan8842_ptp_probe_once(struct phy_device *phydev)
+static int lan8842_ptp_probe(struct phy_device *phydev)
{
- return __lan8814_ptp_probe_once(phydev, "lan8842_ptp_pin",
+ return lan8814_ptp_join(phydev, "lan8842_ptp_pin",
LAN8842_PTP_GPIO_NUM);
}
@@ -6213,11 +6314,9 @@ static int lan8842_probe(struct phy_device *phydev)
if (ret)
return ret;
- if (phy_package_init_once(phydev)) {
- ret = lan8842_ptp_probe_once(phydev);
- if (ret)
- return ret;
- }
+ ret = lan8842_ptp_probe(phydev);
+ if (ret)
+ return ret;
lan8814_ptp_init(phydev);
@@ -6955,6 +7054,7 @@ static struct phy_driver ksphy_driver[] = {
.config_init = lan8814_config_init,
.driver_data = &lan8814_type,
.probe = lan8814_probe,
+ .remove = lan8814_remove,
.soft_reset = genphy_soft_reset,
.read_status = ksz9031_read_status,
.get_sset_count = kszphy_get_sset_count,
@@ -7006,6 +7106,7 @@ static struct phy_driver ksphy_driver[] = {
.flags = PHY_POLL_CABLE_TEST,
.driver_data = &lan8814_type,
.probe = lan8842_probe,
+ .remove = lan8814_remove,
.config_init = lan8842_config_init,
.config_intr = lan8842_config_intr,
.inband_caps = lan8842_inband_caps,
base-commit: 787268686c64be60023743b527645a907e38ccca
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net] net: phy: micrel: Fix potential shared PHC use-after-free
2026-11-07 6:54 [PATCH net] net: phy: micrel: Fix potential shared PHC use-after-free Guangshuo Li
@ 2026-11-07 14:40 ` Andrew Lunn
2026-11-08 7:15 ` netdev-bot+sashiko
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Lunn @ 2026-11-07 14:40 UTC (permalink / raw)
To: Guangshuo Li
Cc: Heiner Kallweit, Russell King, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, netdev, linux-kernel
On Sat, Nov 07, 2026 at 02:54:37PM +0800, Guangshuo Li wrote:
> The shared PHC outlives the first PHY's managed pin array and is not
> unregistered before the package data is freed. Subsequent PHC operations
> can therefore access released storage.
>
> Keep the pins at package scope and select a remaining PHY for register
> access when a member leaves. Unregister the clock before releasing its
> storage on the last member's exit. Pair each member's registration with
> cleanup that also runs on probe failure, and serialize clock access with
> member removal.
You fix looks overly complicated. Ugly.
Try looking at it from a different direction.
.probe() and .remove() should be opposites of each other. What happens
if you do that?
Andrew
---
pw-bot: cr
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net] net: phy: micrel: Fix potential shared PHC use-after-free
2026-11-07 6:54 [PATCH net] net: phy: micrel: Fix potential shared PHC use-after-free Guangshuo Li
2026-11-07 14:40 ` Andrew Lunn
@ 2026-11-08 7:15 ` netdev-bot+sashiko
1 sibling, 0 replies; 3+ messages in thread
From: netdev-bot+sashiko @ 2026-11-08 7:15 UTC (permalink / raw)
To: liguangshuo
Cc: andrew, hkallweit1, linux, davem, edumazet, kuba, pabeni, netdev,
linux-kernel
Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.
Critical: 0 · High: 1 (1) · Medium: 2 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [High] lan8814_ts_info() now calls mutex_lock(&shared->shared_lock),
which can sleep, but the ethtool core always calls this callback…
- [Medium] After a member leaves, the shared PHC stays registered as a
device-model child of the first PHY's mdio.dev.
- [Medium] lan8814_ptp_leave() uses shared->clock_stopping as a gate for
a code region.
Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but the same
driver's LAN8841 PHC has exactly the use-after-free this commit…
--
net: phy: micrel: Fix potential shared PHC use-after-free
The patch moves the LAN8814/LAN8842 shared PHC pin array out of devm and
into package scope, and tracks which package members are bound. When a
member leaves, register access moves to a remaining PHY. The clock is
unregistered when the last member leaves, and shared_lock now also guards
ts_info and the GPIO interrupt path.
> diff --git a/drivers/net/phy/micrel.c b/drivers/net/phy/micrel.c
> index d0b1ad21667b6..8c48d00b0ccf6 100644
> --- a/drivers/net/phy/micrel.c
> +++ b/drivers/net/phy/micrel.c
[ ... ]
> @@ -3257,7 +3261,13 @@ static int lan8814_ts_info(struct mii_timestamper *mii_ts, struct kernel_ethtool
> SOF_TIMESTAMPING_RX_HARDWARE |
> SOF_TIMESTAMPING_RAW_HARDWARE;
>
> + mutex_lock(&shared->shared_lock);
> + if (!shared->ptp_clock || shared->clock_stopping) {
> + mutex_unlock(&shared->shared_lock);
> + return -ENODEV;
> + }
> info->phc_index = ptp_clock_index(shared->ptp_clock);
> + mutex_unlock(&shared->shared_lock);
[Severity: High]
Can this mutex_lock() sleep inside an RCU read-side critical section? The
ethtool core calls the ts_info callback with rcu_read_lock() held:
net/ethtool/common.c:__ethtool_get_ts_info() {
rcu_read_lock();
hwprov = rcu_dereference(dev->hwprov);
...
if (phy_is_default_hwtstamp(phydev) &&
phy_has_tsinfo(phydev)) {
err = phy_ts_info(phydev, info);
...
err = ethtool_get_ts_info_by_phc(dev, info, &hwprov->desc);
rcu_read_unlock();
return err;
}
phy_ts_info() calls phydev->mii_ts->ts_info() directly. For LAN8814 and
LAN8842 that callback is lan8814_ts_info().
shared_lock is also held across slow MDIO accesses by gettime64, adjfine,
adjtime and the GPIO interrupt thread. While ptp4l or phc2sys is running,
this path can block for real.
Any user can reach it with ethtool -T, ETHTOOL_MSG_TSINFO_GET, the
ETHTOOL_GET_TS_INFO ioctl, or SO_TIMESTAMPING with
SOF_TIMESTAMPING_BIND_PHC. With CONFIG_DEBUG_ATOMIC_SLEEP or PROVE_RCU,
won't each call print a "sleeping function called from invalid context"
splat? On non-preemptible RCU kernels, won't it hit "scheduling while
atomic"?
[ ... ]
> @@ -4636,12 +4649,90 @@ static int __lan8814_ptp_probe_once(struct phy_device *phydev, char *pin_name,
> return 0;
> }
>
> -static int lan8814_ptp_probe_once(struct phy_device *phydev)
> +static void lan8814_ptp_leave(void *data)
> +{
> + struct phy_device *phydev = data;
> + struct lan8814_shared_priv *shared = phy_package_get_priv(phydev);
> + struct phy_device *next = NULL;
> + unsigned int i;
> +
> + phy_package_lock(phydev);
> + mutex_lock(&shared->shared_lock);
> + shared->members[phydev->mdio.addr] = NULL;
> + for (i = 0; i < ARRAY_SIZE(shared->members); i++) {
> + if (shared->members[i]) {
> + next = shared->members[i];
> + break;
> + }
> + }
> + if (next) {
> + shared->phydev = next;
> + mutex_unlock(&shared->shared_lock);
> + phy_package_unlock(phydev);
> + return;
> + }
[Severity: Medium]
This handoff only switches shared->phydev. The PHC device is still a child
of the PHY that registered it, because lan8814_ptp_register() does:
shared->ptp_clock = ptp_clock_register(&shared->ptp_clock_info,
&phydev->mdio.dev);
and ptp_clock_register() sets ptp->dev.parent once, at registration time.
mdiobus_unregister() removes PHYs in ascending address order. For the
registering PHY, phy_device_remove()->device_del(&phydev->mdio.dev) first
runs lan8814_ptp_leave() and takes this handoff path. It then deletes the
device and its sysfs subtree, including the ptpN directory with its pins
and n_vclocks attributes, while /dev/ptpN is still live.
The last member then calls ptp_clock_unregister()->device_del() on a child
whose parent has already been deleted.
Is this parent-before-child removal order intended? After a sysfs unbind
of the first PHY, the PHC also stays parented under a PHY with no driver
bound, so the ptpN to device link userspace sees is wrong.
I don't see a crash from this. sysfs_remove_group() only pr_debug()s, and
the child still holds a reference on the parent kobject. But isn't the
PHC's device-model lifetime still tied to the first member, even though
the commit is meant to fix PHC lifetime across package members?
> +
> + shared->clock_stopping = true;
> + mutex_unlock(&shared->shared_lock);
> + /* Unregister calls enable() to disable events, using the last PHY. */
> + ptp_clock_unregister(shared->ptp_clock);
> + mutex_lock(&shared->shared_lock);
> + shared->ptp_clock = NULL;
> + shared->phydev = NULL;
> + kfree(shared->pin_config);
> + shared->pin_config = NULL;
> + shared->clock_stopping = false;
> + mutex_unlock(&shared->shared_lock);
> + phy_package_unlock(phydev);
> +}
[Severity: Medium]
Is shared->clock_stopping acting as an ad-hoc lock here? It is set before
the unlocked ptp_clock_unregister() call and cleared after it.
lan8814_ts_info() and lan8814_handle_gpio_interrupt() check it to avoid
shared->ptp_clock during that window. Lockdep can't see this gate.
The flag seems to exist because shared_lock can't be held across
ptp_clock_unregister(): ptp_disable_all_events() calls info->enable() ->
lan8814_ptp_extts(), which takes shared_lock.
One side effect: an EXTTS capture interrupt can arrive in this window,
before ptp_disable_all_events() clears PTP_COMMON_INT_ENA_GPIO_CAP_EN.
lan8814_handle_gpio_interrupt() then skips the capture:
if (shared->ptp_clock && !shared->clock_stopping)
ret = lan8814_gpio_process_cap(shared);
else
ret = -ENODEV;
PTP_GPIO_CAP_STS is never read, and lan8814_handle_interrupt() and
lan8842_handle_interrupt() treat the -ENODEV as not handled.
lan8814_ts_info() also returns -ENODEV during the window.
Could this teardown use a real lock or RCU instead of a flag?
[ ... ]
> @@ -7006,6 +7106,7 @@ static struct phy_driver ksphy_driver[] = {
> .flags = PHY_POLL_CABLE_TEST,
> .driver_data = &lan8814_type,
> .probe = lan8842_probe,
> + .remove = lan8814_remove,
> .config_init = lan8842_config_init,
> .config_intr = lan8842_config_intr,
> .inband_caps = lan8842_inband_caps,
[Severity: High]
This isn't a bug introduced by this patch, but the LAN8841 PHC in the same
driver seems to have the same use-after-free this commit describes, and it
is left unfixed.
lan8841_probe() allocates ptp_priv->pin_config with
devm_kcalloc(&phydev->mdio.dev, ...). ptp_priv->ptp_clock_info is embedded
in the devm-allocated kszphy_priv. The clock is registered with:
ptp_priv->ptp_clock = ptp_clock_register(&ptp_priv->ptp_clock_info,
&phydev->mdio.dev);
The LAN8841 phy_driver entry has no .remove, and no devm action is added.
After this patch, the only ptp_clock_unregister() in micrel.c is the one
in lan8814_ptp_leave().
When a LAN8841 PHY is unbound (sysfs unbind, or MAC/MDIO bus removal),
devres frees priv and pin_config. /dev/ptpN stays registered with
ptp->info pointing into the freed memory.
Wouldn't a later PTP_CLOCK_GETCAPS, a clock_gettime() through
info->gettime64, or a pin sysfs access then use freed memory? A ptp4l that
still holds the fd open would hit this.
Should LAN8841 get the same treatment, either in this patch or in a
follow-up?
--
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261107-0d0c29e10a-net-micrel-lan8814-1%40mail.sdu.edu.cn
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-11-08 7:15 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-07 6:54 [PATCH net] net: phy: micrel: Fix potential shared PHC use-after-free Guangshuo Li
2026-11-07 14:40 ` Andrew Lunn
2026-11-08 7:15 ` netdev-bot+sashiko
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®