[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20260122041908.2602647-2-kuba@kernel.org>
Date: Wed, 21 Jan 2026 20:19:09 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: kiran.kella@...adcom.com
Cc: Jakub Kicinski <kuba@...nel.org>,
davem@...emloft.net,
ajit.khaparde@...adcom.com,
jayakrishnan.udayavarma@...adcom.com,
netdev@...r.kernel.org,
akhilesh.samineni@...adcom.com,
pabeni@...hat.com,
edumazet@...gle.com,
andrew+netdev@...n.ch,
linux-kernel@...r.kernel.org,
horms@...nel.org
Subject: Re: [net-next,1/2] psp: Support for transmit on logical device when the underlying transport device supports PSP.
Haven't had the time to think about this TBH but FWIW:
This is an AI-generated review of your patch.
For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
psp: Support for transmit on logical device when the underlying transport
device supports PSP.
This patch propagates psp_dev pointers from lower devices to upper devices
(e.g., VLANs) via a netdevice notifier. Upper devices borrow the pointer
without taking a reference, and the borrowed pointer is cleared when the
link changes.
> diff --git a/net/psp/psp_main.c b/net/psp/psp_main.c
> index a8534124f626..0bff50c9314d 100644
> --- a/net/psp/psp_main.c
> +++ b/net/psp/psp_main.c
> @@ -313,10 +313,124 @@ int psp_dev_rcv(struct sk_buff *skb, u16 dev_id, u8 generation, bool strip_icv)
> }
> EXPORT_SYMBOL(psp_dev_rcv);
>
> +/**
> + * psp_netdevice_event() - Handle netdevice events for PSP device propagation
[ ... ]
> + * Propagates psp_dev pointer from lower devices to upper devices when
> + * upper devices are created (e.g., VLAN subinterfaces).
> + */
> +static int psp_netdevice_event(struct notifier_block *nb,
> + unsigned long event, void *ptr)
> +{
[ ... ]
> + /* Get psp_dev from the immediate lower device */
> + psd = rcu_dereference(dev->psp_dev);
> + rcu_read_unlock();
> +
> + /* Propagate psp_dev to upper device if found */
> + if (psd)
> + rcu_assign_pointer(upper_dev->psp_dev, psd);
The commit message states that borrowed pointers are cleared when the lower
device is "unlinked". However, what happens if psp_dev_unregister() is
called on the lower device without first unlinking the upper devices?
Looking at nsim_psp_rereg_write() in drivers/net/netdevsim/psp.c, it calls
nsim_psp_uninit()->psp_dev_unregister() without unregistering the netdevice.
If there are upper devices (e.g., VLANs) that have borrowed the psp_dev
pointer, would they be left with a dangling pointer after the RCU grace
period completes?
Similarly, nsim_destroy() calls nsim_psp_uninit() before calling
unregister_netdevice(), so the upper devices would still have borrowed
pointers when psp_dev_unregister() frees the psp_dev structure.
Could this lead to use-after-free if an upper device's psp_dev pointer is
accessed after the underlying psp_dev has been freed?
Would it make sense to either:
(a) Document that netdevice must be unregistered before psp_dev_unregister()
is called, or
(b) Walk the upper device stack in psp_dev_unregister() to clear borrowed
pointers, or
(c) Update existing callers (netdevsim) to ensure proper ordering?
> + } else {
[ ... ]
> + return NOTIFY_DONE;
> +}
--
pw-bot: cr
Powered by blists - more mailing lists