Netdev List
 help / color / mirror / Atom feed
* [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; 3+ 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] 3+ 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
  0 siblings, 1 reply; 3+ 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] 3+ 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
  0 siblings, 0 replies; 3+ 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] 3+ messages in thread

end of thread, other threads:[~2026-07-06 16:32 UTC | newest]

Thread overview: 3+ 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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox