[PATCH net v1] pfcp: fix socket lifetime on netdevice registration failure

[PATCH net v1] pfcp: fix socket lifetime on netdevice registration failure

[Xuanqiang Luo]

From: Xuanqiang Luo <luoxuanqiang@kylinos.cn>

pfcp_newlink() creates the UDP socket before register_netdevice().
If registration fails after pfcp_dev_init() succeeds, the core calls
pfcp_dev_uninit(), which releases the socket and clears pfcp->sk.
The newlink error path then releases it again, causing a NULL pointer
dereference. This was observed with failslab fault injection:

  FAULT_INJECTION: forcing a failure.
  kobject: kobject_add_internal failed for pfcp0 (error: -12 parent: net)
  BUG: KASAN: null-ptr-deref in udp_tunnel_sock_release+0x1c/0x50
  Read of size 8 at addr 0000000000000120 by task ip/1037
  Call trace:
   show_stack+0x18/0x24 (C)
   dump_stack_lvl+0x78/0x90
   print_report+0x468/0x5cc
   kasan_report+0xa4/0xf0
   __asan_load8+0x7c/0xd0
   udp_tunnel_sock_release+0x1c/0x50
   pfcp_newlink+0x128/0x184
   rtnl_newlink+0x848/0xe44
   rtnetlink_rcv_msg+0x468/0x514
   netlink_rcv_skb+0xc0/0x1f0
   rtnetlink_rcv+0x18/0x24
   netlink_unicast+0x4b8/0x558
   netlink_sendmsg+0x2b8/0x584
   ...

Create the socket in pfcp_dev_init() after initializing the GRO cells,
and let pfcp_dev_uninit() release it on registration failure or removal.

Move the RCU wait into pfcp_dev_uninit(), using synchronize_net() after
socket release to drain receive callbacks before destroying the GRO cells.

Fixes: 76c8764ef36a5 ("pfcp: add PFCP module")
Signed-off-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
---
A NULL check would fix the crash. Moving socket creation into ndo_init
also removes the duplicate cleanup and ensures the receive state is
initialized before enabling the socket's receive callback.

 drivers/net/pfcp.c | 90 ++++++++++++++++++++++------------------------
 1 file changed, 42 insertions(+), 48 deletions(-)

diff --git a/drivers/net/pfcp.c b/drivers/net/pfcp.c
index e1cca779d2ecc..3db813ba2344c 100644
--- a/drivers/net/pfcp.c
+++ b/drivers/net/pfcp.c
@@ -103,27 +103,63 @@ static int pfcp_encap_recv(struct sock *sk, struct sk_buff *skb)
 	return 0;
 }
 
-static void pfcp_del_sock(struct pfcp_dev *pfcp)
+static struct sock *pfcp_create_sock(struct pfcp_dev *pfcp)
 {
-	udp_tunnel_sock_release(pfcp->sk);
-	pfcp->sk = NULL;
+	struct udp_tunnel_sock_cfg tuncfg = {};
+	struct udp_port_cfg udp_conf = {
+		.local_ip.s_addr	= htonl(INADDR_ANY),
+		.family			= AF_INET,
+	};
+	struct net *net = pfcp->net;
+	struct socket *sock;
+	int err;
+
+	udp_conf.local_udp_port = htons(PFCP_PORT);
+
+	err = udp_sock_create(net, &udp_conf, &sock);
+	if (err)
+		return ERR_PTR(err);
+
+	tuncfg.sk_user_data = pfcp;
+	tuncfg.encap_rcv = pfcp_encap_recv;
+	tuncfg.encap_type = 1;
+
+	setup_udp_tunnel_sock(net, sock->sk, &tuncfg);
+
+	return sock->sk;
 }
 
 static void pfcp_dev_uninit(struct net_device *dev)
 {
 	struct pfcp_dev *pfcp = netdev_priv(dev);
 
+	udp_tunnel_sock_release(pfcp->sk);
+	pfcp->sk = NULL;
+	/* Wait for receive callbacks before destroying their state. */
+	synchronize_net();
 	gro_cells_destroy(&pfcp->gro_cells);
-	pfcp_del_sock(pfcp);
 }
 
 static int pfcp_dev_init(struct net_device *dev)
 {
 	struct pfcp_dev *pfcp = netdev_priv(dev);
+	struct sock *sk;
+	int err;
 
 	pfcp->dev = dev;
 
-	return gro_cells_init(&pfcp->gro_cells, dev);
+	err = gro_cells_init(&pfcp->gro_cells, dev);
+	if (err)
+		return err;
+
+	sk = pfcp_create_sock(pfcp);
+	if (IS_ERR(sk)) {
+		gro_cells_destroy(&pfcp->gro_cells);
+		return PTR_ERR(sk);
+	}
+	pfcp->sk = sk;
+
+	return 0;
 }
 
 static const struct net_device_ops pfcp_netdev_ops = {
@@ -153,39 +189,6 @@ static void pfcp_link_setup(struct net_device *dev)
 	netif_keep_dst(dev);
 }
 
-static struct sock *pfcp_create_sock(struct pfcp_dev *pfcp)
-{
-	struct udp_tunnel_sock_cfg tuncfg = {};
-	struct udp_port_cfg udp_conf = {
-		.local_ip.s_addr	= htonl(INADDR_ANY),
-		.family			= AF_INET,
-	};
-	struct net *net = pfcp->net;
-	struct socket *sock;
-	int err;
-
-	udp_conf.local_udp_port = htons(PFCP_PORT);
-
-	err = udp_sock_create(net, &udp_conf, &sock);
-	if (err)
-		return ERR_PTR(err);
-
-	tuncfg.sk_user_data = pfcp;
-	tuncfg.encap_rcv = pfcp_encap_recv;
-	tuncfg.encap_type = 1;
-
-	setup_udp_tunnel_sock(net, sock->sk, &tuncfg);
-
-	return sock->sk;
-}
-
-static int pfcp_add_sock(struct pfcp_dev *pfcp)
-{
-	pfcp->sk = pfcp_create_sock(pfcp);
-
-	return PTR_ERR_OR_ZERO(pfcp->sk);
-}
-
 static int pfcp_newlink(struct net_device *dev,
 			struct rtnl_newlink_params *params,
 			struct netlink_ext_ack *extack)
@@ -197,16 +200,10 @@ static int pfcp_newlink(struct net_device *dev,
 
 	pfcp->net = link_net;
 
-	err = pfcp_add_sock(pfcp);
-	if (err) {
-		netdev_dbg(dev, "failed to add pfcp socket %d\n", err);
-		goto exit_err;
-	}
-
 	err = register_netdevice(dev);
 	if (err) {
 		netdev_dbg(dev, "failed to register pfcp netdev %d\n", err);
-		goto exit_del_pfcp_sock;
+		goto exit_err;
 	}
 
 	pn = net_generic(link_net, pfcp_net_id);
@@ -219,9 +216,6 @@ static int pfcp_newlink(struct net_device *dev,
 
 	return 0;
 
-exit_del_pfcp_sock:
-	pfcp_del_sock(pfcp);
-	synchronize_rcu();
 exit_err:
 	pfcp->net = NULL;
 	return err;
-- 
2.43.0

Re: [PATCH net v1] pfcp: fix socket lifetime on netdevice registration failure

[Simon Horman]

On Fri, Sep 18, 2026 at 08:31:58PM +0800, Xuanqiang Luo wrote:
> From: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
> 
> pfcp_newlink() creates the UDP socket before register_netdevice().
> If registration fails after pfcp_dev_init() succeeds, the core calls
> pfcp_dev_uninit(), which releases the socket and clears pfcp->sk.
> The newlink error path then releases it again, causing a NULL pointer
> dereference. This was observed with failslab fault injection:
> 
>   FAULT_INJECTION: forcing a failure.
>   kobject: kobject_add_internal failed for pfcp0 (error: -12 parent: net)
>   BUG: KASAN: null-ptr-deref in udp_tunnel_sock_release+0x1c/0x50
>   Read of size 8 at addr 0000000000000120 by task ip/1037
>   Call trace:
>    show_stack+0x18/0x24 (C)
>    dump_stack_lvl+0x78/0x90
>    print_report+0x468/0x5cc
>    kasan_report+0xa4/0xf0
>    __asan_load8+0x7c/0xd0
>    udp_tunnel_sock_release+0x1c/0x50
>    pfcp_newlink+0x128/0x184
>    rtnl_newlink+0x848/0xe44
>    rtnetlink_rcv_msg+0x468/0x514
>    netlink_rcv_skb+0xc0/0x1f0
>    rtnetlink_rcv+0x18/0x24
>    netlink_unicast+0x4b8/0x558
>    netlink_sendmsg+0x2b8/0x584
>    ...
> 
> Create the socket in pfcp_dev_init() after initializing the GRO cells,
> and let pfcp_dev_uninit() release it on registration failure or removal.
> 
> Move the RCU wait into pfcp_dev_uninit(), using synchronize_net() after
> socket release to drain receive callbacks before destroying the GRO cells.
> 
> Fixes: 76c8764ef36a5 ("pfcp: add PFCP module")
> Signed-off-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
> ---
> A NULL check would fix the crash. Moving socket creation into ndo_init
> also removes the duplicate cleanup and ensures the receive state is
> initialized before enabling the socket's receive callback.

Assuming the NULL check is significantly simpler than this patch,
and that it resolves the bug, think that we should consider:

1. A patch that minimal NULL check for net
2. Follow-up with the approach taken by this patch in net-next

I say this because this seems to be lower risk than skipping to applying
2 to net.

Also, please consider CCing stable on bug fixes.

Link: https://docs.kernel.org/process/maintainer-netdev.html#stable-tree

Re: [PATCH net v1] pfcp: fix socket lifetime on netdevice registration failure

[Xuanqiang Luo]

On 2026/9/23 at 17:29 Simon Horman wrote:
> On Fri, Sep 18, 2026 at 08:31:58PM +0800, Xuanqiang Luo wrote:
>> From: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
>>
>> pfcp_newlink() creates the UDP socket before register_netdevice().
>> If registration fails after pfcp_dev_init() succeeds, the core calls
>> pfcp_dev_uninit(), which releases the socket and clears pfcp->sk.
>> The newlink error path then releases it again, causing a NULL pointer
>> dereference. This was observed with failslab fault injection:
>>
>>    FAULT_INJECTION: forcing a failure.
>>    kobject: kobject_add_internal failed for pfcp0 (error: -12 parent: net)
>>    BUG: KASAN: null-ptr-deref in udp_tunnel_sock_release+0x1c/0x50
>>    Read of size 8 at addr 0000000000000120 by task ip/1037
>>    Call trace:
>>     show_stack+0x18/0x24 (C)
>>     dump_stack_lvl+0x78/0x90
>>     print_report+0x468/0x5cc
>>     kasan_report+0xa4/0xf0
>>     __asan_load8+0x7c/0xd0
>>     udp_tunnel_sock_release+0x1c/0x50
>>     pfcp_newlink+0x128/0x184
>>     rtnl_newlink+0x848/0xe44
>>     rtnetlink_rcv_msg+0x468/0x514
>>     netlink_rcv_skb+0xc0/0x1f0
>>     rtnetlink_rcv+0x18/0x24
>>     netlink_unicast+0x4b8/0x558
>>     netlink_sendmsg+0x2b8/0x584
>>     ...
>>
>> Create the socket in pfcp_dev_init() after initializing the GRO cells,
>> and let pfcp_dev_uninit() release it on registration failure or removal.
>>
>> Move the RCU wait into pfcp_dev_uninit(), using synchronize_net() after
>> socket release to drain receive callbacks before destroying the GRO cells.
>>
>> Fixes: 76c8764ef36a5 ("pfcp: add PFCP module")
>> Signed-off-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
>> ---
>> A NULL check would fix the crash. Moving socket creation into ndo_init
>> also removes the duplicate cleanup and ensures the receive state is
>> initialized before enabling the socket's receive callback.
> 
> Assuming the NULL check is significantly simpler than this patch,
> and that it resolves the bug, think that we should consider:
> 
> 1. A patch that minimal NULL check for net
> 2. Follow-up with the approach taken by this patch in net-next
> 
> I say this because this seems to be lower risk than skipping to applying
> 2 to net.
> 
> Also, please consider CCing stable on bug fixes.
> 
> Link: https://docs.kernel.org/process/maintainer-netdev.html#stable-tree

That makes sense. I left stable off Cc because the lifecycle changes
seemed too broad for a stable fix. Splitting the two makes sense now.

I'll send a v2 for net with just the NULL check and Cc stable, then
follow up with the lifecycle changes once the fix reaches net-next.

Thanks for the suggestion!!

Xuanqiang