* [PATCH net] psp: fix NULL genl_sock deref race with concurrent netns teardown
@ 2026-07-03 11:24 Kiran Kella
2026-07-06 13:46 ` Daniel Zahka
0 siblings, 1 reply; 4+ messages in thread
From: Kiran Kella @ 2026-07-03 11:24 UTC (permalink / raw)
To: daniel.zahka, kuba, willemdebruijn.kernel
Cc: davem, edumazet, pabeni, horms, weibunny, netdev, linux-kernel,
jayakrishnan.udayavarma, ajit.khaparde, akhilesh.samineni,
Kiran Kella, Vikas Gupta, Bhargava Marreddy
The race occurs between network namespace removal and PSP device
unregistration. When a netns is deleted while a PSP device associated
with that netns is concurrently being removed, psp_dev_unregister()
triggers psp_nl_notify_dev() to send a device change notification.
Concurrently, cleanup_net() running in the netns workqueue calls
genl_pernet_exit(), which sets net->genl_sock to NULL. If
genl_pernet_exit() wins the race, two sites in psp_nl_multicast_per_ns()
then dereference the NULL socket and crash:
CPU 0 (netns teardown) CPU 1 (PSP device unregister)
====================== =============================
cleanup_net [workqueue]
genl_pernet_exit() psp_dev_unregister()
net->genl_sock = NULL psp_nl_notify_dev()
psp_nl_multicast_per_ns()
build_ntf()
-> netlink_has_listeners(NULL)
/* crash */
genlmsg_multicast_netns()
-> nlmsg_multicast_filtered(NULL)
/* crash */
Both the main_net path (derived from psd->main_netdev) and each
assoc_net entry in psd->assoc_dev_list are affected.
Fix by replacing the bare dev_net() calls with maybe_get_net().
maybe_get_net() returns NULL if the namespace is already dying.
Holding the reference ensures genl_sock remains valid across both the
build_ntf() and genlmsg_multicast_netns() calls.
Fixes: 00c94ca2b99e ("psp: base PSP device support")
Fixes: 06c2dce2d0f6 ("psp: add new netlink cmd for dev-assoc and dev-disassoc")
Signed-off-by: Kiran Kella <kiran.kella@broadcom.com>
Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
Reviewed-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
---
net/psp/psp_nl.c | 33 +++++++++++++++++++++------------
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/net/psp/psp_nl.c b/net/psp/psp_nl.c
index 9610d8c456ff..24ab626a9e8a 100644
--- a/net/psp/psp_nl.c
+++ b/net/psp/psp_nl.c
@@ -62,36 +62,45 @@ psp_nl_multicast_per_ns(struct psp_dev *psd, unsigned int group,
struct net *main_net;
struct sk_buff *ntf;
- main_net = dev_net(psd->main_netdev);
+ main_net = maybe_get_net(dev_net(psd->main_netdev));
+ if (!main_net)
+ return;
+
xa_init(&sent_nets);
list_for_each_entry(entry, &psd->assoc_dev_list, dev_list) {
struct net *assoc_net = dev_net(entry->assoc_dev);
+ struct net *net;
int ret;
if (net_eq(assoc_net, main_net))
continue;
- ret = xa_insert(&sent_nets, (unsigned long)assoc_net, assoc_net,
- GFP_KERNEL);
- if (ret == -EBUSY)
+ net = maybe_get_net(assoc_net);
+ if (!net)
continue;
- ntf = build_ntf(psd, assoc_net, ctx);
- if (!ntf)
+ ret = xa_insert(&sent_nets, (unsigned long)assoc_net, assoc_net,
+ GFP_KERNEL);
+ if (ret == -EBUSY) {
+ put_net(net);
continue;
+ }
- genlmsg_multicast_netns(&psp_nl_family, assoc_net, ntf, 0,
- group, GFP_KERNEL);
+ ntf = build_ntf(psd, net, ctx);
+ if (ntf)
+ genlmsg_multicast_netns(&psp_nl_family, net, ntf, 0,
+ group, GFP_KERNEL);
+ put_net(net);
}
xa_destroy(&sent_nets);
/* Send to main device netns */
ntf = build_ntf(psd, main_net, ctx);
- if (!ntf)
- return;
- genlmsg_multicast_netns(&psp_nl_family, main_net, ntf, 0, group,
- GFP_KERNEL);
+ if (ntf)
+ genlmsg_multicast_netns(&psp_nl_family, main_net, ntf, 0, group,
+ GFP_KERNEL);
+ put_net(main_net);
}
static struct sk_buff *psp_nl_clone_ntf(struct psp_dev *psd, struct net *net,
--
2.54.0
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH net] psp: fix NULL genl_sock deref race with concurrent netns teardown
2026-07-03 11:24 [PATCH net] psp: fix NULL genl_sock deref race with concurrent netns teardown Kiran Kella
@ 2026-07-06 13:46 ` Daniel Zahka
2026-07-06 16:32 ` Wei Wang
2026-07-07 6:23 ` Kiran Kella
0 siblings, 2 replies; 4+ messages in thread
From: Daniel Zahka @ 2026-07-06 13:46 UTC (permalink / raw)
To: Kiran Kella, kuba, willemdebruijn.kernel
Cc: davem, edumazet, pabeni, horms, weibunny, netdev, linux-kernel,
jayakrishnan.udayavarma, ajit.khaparde, akhilesh.samineni,
Vikas Gupta, Bhargava Marreddy
On 7/3/26 7:24 AM, Kiran Kella wrote:
> The race occurs between network namespace removal and PSP device
> unregistration. When a netns is deleted while a PSP device associated
> with that netns is concurrently being removed, psp_dev_unregister()
> triggers psp_nl_notify_dev() to send a device change notification.
> Concurrently, cleanup_net() running in the netns workqueue calls
> genl_pernet_exit(), which sets net->genl_sock to NULL. If
> genl_pernet_exit() wins the race, two sites in psp_nl_multicast_per_ns()
> then dereference the NULL socket and crash:
>
> CPU 0 (netns teardown) CPU 1 (PSP device unregister)
> ====================== =============================
> cleanup_net [workqueue]
> genl_pernet_exit() psp_dev_unregister()
> net->genl_sock = NULL psp_nl_notify_dev()
> psp_nl_multicast_per_ns()
> build_ntf()
> -> netlink_has_listeners(NULL)
> /* crash */
> genlmsg_multicast_netns()
> -> nlmsg_multicast_filtered(NULL)
> /* crash */
>
> Both the main_net path (derived from psd->main_netdev) and each
> assoc_net entry in psd->assoc_dev_list are affected.
>
> Fix by replacing the bare dev_net() calls with maybe_get_net().
> maybe_get_net() returns NULL if the namespace is already dying.
> Holding the reference ensures genl_sock remains valid across both the
> build_ntf() and genlmsg_multicast_netns() calls.
>
> Fixes: 00c94ca2b99e ("psp: base PSP device support")
> Fixes: 06c2dce2d0f6 ("psp: add new netlink cmd for dev-assoc and dev-disassoc")
> Signed-off-by: Kiran Kella <kiran.kella@broadcom.com>
> Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
> Reviewed-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
Thanks. I was able to repro myself by inserting a delay after the old:
main_net = dev_net(psd->main_netdev);
and then executing nsim_psp_rereg_write() in parallel with destroying
the netns that I placed a netdevsim dev into.
Tested-by: Daniel Zahka <daniel.zahka@gmail.com>
Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>
> ---
> net/psp/psp_nl.c | 33 +++++++++++++++++++++------------
> 1 file changed, 21 insertions(+), 12 deletions(-)
>
> diff --git a/net/psp/psp_nl.c b/net/psp/psp_nl.c
> index 9610d8c456ff..24ab626a9e8a 100644
> --- a/net/psp/psp_nl.c
> +++ b/net/psp/psp_nl.c
> @@ -62,36 +62,45 @@ psp_nl_multicast_per_ns(struct psp_dev *psd, unsigned int group,
> struct net *main_net;
> struct sk_buff *ntf;
>
> - main_net = dev_net(psd->main_netdev);
> + main_net = maybe_get_net(dev_net(psd->main_netdev));
> + if (!main_net)
> + return;
> +
> xa_init(&sent_nets);
>
> list_for_each_entry(entry, &psd->assoc_dev_list, dev_list) {
> struct net *assoc_net = dev_net(entry->assoc_dev);
> + struct net *net;
> int ret;
>
> if (net_eq(assoc_net, main_net))
> continue;
>
> - ret = xa_insert(&sent_nets, (unsigned long)assoc_net, assoc_net,
> - GFP_KERNEL);
> - if (ret == -EBUSY)
> + net = maybe_get_net(assoc_net);
> + if (!net)
> continue;
>
> - ntf = build_ntf(psd, assoc_net, ctx);
> - if (!ntf)
> + ret = xa_insert(&sent_nets, (unsigned long)assoc_net, assoc_net,
> + GFP_KERNEL);
> + if (ret == -EBUSY) {
> + put_net(net);
> continue;
> + }
>
> - genlmsg_multicast_netns(&psp_nl_family, assoc_net, ntf, 0,
> - group, GFP_KERNEL);
> + ntf = build_ntf(psd, net, ctx);
> + if (ntf)
> + genlmsg_multicast_netns(&psp_nl_family, net, ntf, 0,
> + group, GFP_KERNEL);
> + put_net(net);
> }
some optional nits if you wanted to respin:
You could eliminate the extra struct net *net, by just doing something
like if (!maybe_get_net(assoc_net)) directly.
You could get away with a single put_net(net); call site, if you reorder
the ops that could fail so that maybe_get_net(assoc_net) happens last
before the build_ntf(psd, net, ctx)
> xa_destroy(&sent_nets);
>
> /* Send to main device netns */
> ntf = build_ntf(psd, main_net, ctx);
> - if (!ntf)
> - return;
> - genlmsg_multicast_netns(&psp_nl_family, main_net, ntf, 0, group,
> - GFP_KERNEL);
> + if (ntf)
> + genlmsg_multicast_netns(&psp_nl_family, main_net, ntf, 0, group,
> + GFP_KERNEL);
> + put_net(main_net);
> }
>
> static struct sk_buff *psp_nl_clone_ntf(struct psp_dev *psd, struct net *net,
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net] psp: fix NULL genl_sock deref race with concurrent netns teardown
2026-07-06 13:46 ` Daniel Zahka
@ 2026-07-06 16:32 ` Wei Wang
2026-07-07 6:23 ` Kiran Kella
1 sibling, 0 replies; 4+ messages in thread
From: Wei Wang @ 2026-07-06 16:32 UTC (permalink / raw)
To: Daniel Zahka
Cc: Kiran Kella, kuba, willemdebruijn.kernel, davem, edumazet, pabeni,
horms, weibunny, netdev, linux-kernel, jayakrishnan.udayavarma,
ajit.khaparde, akhilesh.samineni, Vikas Gupta, Bhargava Marreddy
On Mon, Jul 6, 2026 at 6:46 AM Daniel Zahka <daniel.zahka@gmail.com> wrote:
>
> >
>
> On 7/3/26 7:24 AM, Kiran Kella wrote:
> > The race occurs between network namespace removal and PSP device
> > unregistration. When a netns is deleted while a PSP device associated
> > with that netns is concurrently being removed, psp_dev_unregister()
> > triggers psp_nl_notify_dev() to send a device change notification.
> > Concurrently, cleanup_net() running in the netns workqueue calls
> > genl_pernet_exit(), which sets net->genl_sock to NULL. If
> > genl_pernet_exit() wins the race, two sites in psp_nl_multicast_per_ns()
> > then dereference the NULL socket and crash:
> >
> > CPU 0 (netns teardown) CPU 1 (PSP device unregister)
> > ====================== =============================
> > cleanup_net [workqueue]
> > genl_pernet_exit() psp_dev_unregister()
> > net->genl_sock = NULL psp_nl_notify_dev()
> > psp_nl_multicast_per_ns()
> > build_ntf()
> > -> netlink_has_listeners(NULL)
> > /* crash */
> > genlmsg_multicast_netns()
> > -> nlmsg_multicast_filtered(NULL)
> > /* crash */
> >
> > Both the main_net path (derived from psd->main_netdev) and each
> > assoc_net entry in psd->assoc_dev_list are affected.
> >
> > Fix by replacing the bare dev_net() calls with maybe_get_net().
> > maybe_get_net() returns NULL if the namespace is already dying.
> > Holding the reference ensures genl_sock remains valid across both the
> > build_ntf() and genlmsg_multicast_netns() calls.
> >
> > Fixes: 00c94ca2b99e ("psp: base PSP device support")
> > Fixes: 06c2dce2d0f6 ("psp: add new netlink cmd for dev-assoc and dev-disassoc")
> > Signed-off-by: Kiran Kella <kiran.kella@broadcom.com>
> > Reviewed-by: Ajit Khaparde <ajit.khaparde@broadcom.com>
> > Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
> > Reviewed-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
>
>
> Thanks. I was able to repro myself by inserting a delay after the old:
>
> main_net = dev_net(psd->main_netdev);
>
> and then executing nsim_psp_rereg_write() in parallel with destroying
> the netns that I placed a netdevsim dev into.
>
> Tested-by: Daniel Zahka <daniel.zahka@gmail.com>
>
> Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>
>
Thanks for the fix!
Reviewed-by: Wei Wang <weibunny@fb.com>
> > ---
> > net/psp/psp_nl.c | 33 +++++++++++++++++++++------------
> > 1 file changed, 21 insertions(+), 12 deletions(-)
> >
> > diff --git a/net/psp/psp_nl.c b/net/psp/psp_nl.c
> > index 9610d8c456ff..24ab626a9e8a 100644
> > --- a/net/psp/psp_nl.c
> > +++ b/net/psp/psp_nl.c
> > @@ -62,36 +62,45 @@ psp_nl_multicast_per_ns(struct psp_dev *psd, unsigned int group,
> > struct net *main_net;
> > struct sk_buff *ntf;
> >
> > - main_net = dev_net(psd->main_netdev);
> > + main_net = maybe_get_net(dev_net(psd->main_netdev));
> > + if (!main_net)
> > + return;
> > +
> > xa_init(&sent_nets);
> >
> > list_for_each_entry(entry, &psd->assoc_dev_list, dev_list) {
> > struct net *assoc_net = dev_net(entry->assoc_dev);
> > + struct net *net;
> > int ret;
> >
> > if (net_eq(assoc_net, main_net))
> > continue;
> >
> > - ret = xa_insert(&sent_nets, (unsigned long)assoc_net, assoc_net,
> > - GFP_KERNEL);
> > - if (ret == -EBUSY)
> > + net = maybe_get_net(assoc_net);
> > + if (!net)
> > continue;
> >
> > - ntf = build_ntf(psd, assoc_net, ctx);
> > - if (!ntf)
> > + ret = xa_insert(&sent_nets, (unsigned long)assoc_net, assoc_net,
> > + GFP_KERNEL);
> > + if (ret == -EBUSY) {
> > + put_net(net);
> > continue;
> > + }
> >
> > - genlmsg_multicast_netns(&psp_nl_family, assoc_net, ntf, 0,
> > - group, GFP_KERNEL);
> > + ntf = build_ntf(psd, net, ctx);
> > + if (ntf)
> > + genlmsg_multicast_netns(&psp_nl_family, net, ntf, 0,
> > + group, GFP_KERNEL);
> > + put_net(net);
> > }
>
> some optional nits if you wanted to respin:
>
> You could eliminate the extra struct net *net, by just doing something
> like if (!maybe_get_net(assoc_net)) directly.
>
> You could get away with a single put_net(net); call site, if you reorder
> the ops that could fail so that maybe_get_net(assoc_net) happens last
> before the build_ntf(psd, net, ctx)
>
> > xa_destroy(&sent_nets);
> >
> > /* Send to main device netns */
> > ntf = build_ntf(psd, main_net, ctx);
> > - if (!ntf)
> > - return;
> > - genlmsg_multicast_netns(&psp_nl_family, main_net, ntf, 0, group,
> > - GFP_KERNEL);
> > + if (ntf)
> > + genlmsg_multicast_netns(&psp_nl_family, main_net, ntf, 0, group,
> > + GFP_KERNEL);
> > + put_net(main_net);
> > }
> >
> > static struct sk_buff *psp_nl_clone_ntf(struct psp_dev *psd, struct net *net,
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net] psp: fix NULL genl_sock deref race with concurrent netns teardown
2026-07-06 13:46 ` Daniel Zahka
2026-07-06 16:32 ` Wei Wang
@ 2026-07-07 6:23 ` Kiran Kella
1 sibling, 0 replies; 4+ messages in thread
From: Kiran Kella @ 2026-07-07 6:23 UTC (permalink / raw)
To: Daniel Zahka
Cc: kuba, willemdebruijn.kernel, davem, edumazet, pabeni, horms,
weibunny, netdev, linux-kernel, jayakrishnan.udayavarma,
ajit.khaparde, akhilesh.samineni, Vikas Gupta, Bhargava Marreddy
[-- Attachment #1: Type: text/plain, Size: 1627 bytes --]
On Mon, Jul 6, 2026 at 7:16 PM Daniel Zahka <daniel.zahka@gmail.com> wrote:
>
>
> On 7/3/26 7:24 AM, Kiran Kella wrote:
> > The race occurs between network namespace removal and PSP device
> > unregistration. When a netns is deleted while a PSP device associated
> > with that netns is concurrently being removed, psp_dev_unregister()
>
>
> Thanks. I was able to repro myself by inserting a delay after the old:
>
> main_net = dev_net(psd->main_netdev);
>
> and then executing nsim_psp_rereg_write() in parallel with destroying
> the netns that I placed a netdevsim dev into.
>
> Tested-by: Daniel Zahka <daniel.zahka@gmail.com>
>
> Reviewed-by: Daniel Zahka <daniel.zahka@gmail.com>
Thanks Daniel for reviewing and validating the fix.
>
> > ---
> > net/psp/psp_nl.c | 33 +++++++++++++++++++++------------
> > 1 file changed, 21 insertions(+), 12 deletions(-)
> > + ntf = build_ntf(psd, net, ctx);
> > + if (ntf)
> > + genlmsg_multicast_netns(&psp_nl_family, net, ntf, 0,
> > + group, GFP_KERNEL);
> > + put_net(net);
> > }
>
> some optional nits if you wanted to respin:
>
> You could eliminate the extra struct net *net, by just doing something
> like if (!maybe_get_net(assoc_net)) directly.
>
> You could get away with a single put_net(net); call site, if you reorder
> the ops that could fail so that maybe_get_net(assoc_net) happens last
> before the build_ntf(psd, net, ctx)
>
ACK. Will fix in v2.
Thanks,
Kiran
> > xa_destroy(&sent_nets);
> >
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 5465 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-07-07 6:23 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-03 11:24 [PATCH net] psp: fix NULL genl_sock deref race with concurrent netns teardown Kiran Kella
2026-07-06 13:46 ` Daniel Zahka
2026-07-06 16:32 ` Wei Wang
2026-07-07 6:23 ` Kiran Kella
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox