[PATCH v1 net 0/3] gtp/pfcp: Fix use-after-free of UDP tunnel socket.

[PATCH v1 net 0/3] gtp/pfcp: Fix use-after-free of UDP tunnel socket.

[Kuniyuki Iwashima]

Xiao Liang pointed out weird netns usages in ->newlink() of
gtp and pfcp.

This series fixes the issues.

Link: https://lore.kernel.org/netdev/20250104125732.17335-1-shaw.leon@gmail.com/


Kuniyuki Iwashima (3):
  gtp: Use for_each_netdev_rcu() in gtp_genl_dump_pdp().
  gtp: Destroy device along with udp socket's netns dismantle.
  pfcp: Destroy device along with udp socket's netns dismantle.

 drivers/net/gtp.c  | 21 +++++++++++++++------
 drivers/net/pfcp.c | 15 ++++++++++-----
 2 files changed, 25 insertions(+), 11 deletions(-)

-- 
2.39.5 (Apple Git-154)

[PATCH v1 net 1/3] gtp: Use for_each_netdev_rcu() in gtp_genl_dump_pdp().

[Kuniyuki Iwashima]

gtp_newlink() links the gtp device to a list in dev_net(dev).

However, even after the gtp device is moved to another netns,
it stays on the list but should be invisible.

Let's use for_each_netdev_rcu() for netdev traversal in
gtp_genl_dump_pdp().

Note that gtp_dev_list is no longer used under RCU, so list
helpers are converted to the non-RCU variant.

Fixes: 459aa660eb1d ("gtp: add initial driver for datapath of GPRS Tunneling Protocol (GTP-U)")
Reported-by: Xiao Liang <shaw.leon@gmail.com>
Closes: https://lore.kernel.org/netdev/CABAhCOQdBL6h9M2C+kd+bGivRJ9Q72JUxW+-gur0nub_=PmFPA@mail.gmail.com/
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Harald Welte <laforge@gnumonks.org>
---
 drivers/net/gtp.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 89a996ad8cd0..8e456d09d173 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -1525,7 +1525,7 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev,
 	}
 
 	gn = net_generic(dev_net(dev), gtp_net_id);
-	list_add_rcu(&gtp->list, &gn->gtp_dev_list);
+	list_add(&gtp->list, &gn->gtp_dev_list);
 	dev->priv_destructor = gtp_destructor;
 
 	netdev_dbg(dev, "registered new GTP interface\n");
@@ -1551,7 +1551,7 @@ static void gtp_dellink(struct net_device *dev, struct list_head *head)
 		hlist_for_each_entry_safe(pctx, next, &gtp->tid_hash[i], hlist_tid)
 			pdp_context_delete(pctx);
 
-	list_del_rcu(&gtp->list);
+	list_del(&gtp->list);
 	unregister_netdevice_queue(dev, head);
 }
 
@@ -2271,6 +2271,7 @@ static int gtp_genl_dump_pdp(struct sk_buff *skb,
 	struct gtp_dev *last_gtp = (struct gtp_dev *)cb->args[2], *gtp;
 	int i, j, bucket = cb->args[0], skip = cb->args[1];
 	struct net *net = sock_net(skb->sk);
+	struct net_device *dev;
 	struct pdp_ctx *pctx;
 	struct gtp_net *gn;
 
@@ -2280,7 +2281,10 @@ static int gtp_genl_dump_pdp(struct sk_buff *skb,
 		return 0;
 
 	rcu_read_lock();
-	list_for_each_entry_rcu(gtp, &gn->gtp_dev_list, list) {
+	for_each_netdev_rcu(net, dev) {
+		if (dev->rtnl_link_ops != &gtp_link_ops)
+			continue;
+
 		if (last_gtp && last_gtp != gtp)
 			continue;
 		else
@@ -2475,9 +2479,9 @@ static void __net_exit gtp_net_exit_batch_rtnl(struct list_head *net_list,
 
 	list_for_each_entry(net, net_list, exit_list) {
 		struct gtp_net *gn = net_generic(net, gtp_net_id);
-		struct gtp_dev *gtp;
+		struct gtp_dev *gtp, *gtp_next;
 
-		list_for_each_entry(gtp, &gn->gtp_dev_list, list)
+		list_for_each_entry_safe(gtp, gtp_next, &gn->gtp_dev_list, list)
 			gtp_dellink(gtp->dev, dev_to_kill);
 	}
 }
-- 
2.39.5 (Apple Git-154)

[PATCH v1 net 2/3] gtp: Destroy device along with udp socket's netns dismantle.

[Kuniyuki Iwashima]

gtp_newlink() links the device to a list in dev_net(dev) instead of
src_net, where a udp tunnel socket is created.

Even when src_net is removed, the device stays alive on dev_net(dev).
Then, removing src_net triggers the splat below. [0]

In this example, gtp0 is created in ns2, and the udp socket is created
in ns1.

  ip netns add ns1
  ip netns add ns2
  ip -n ns1 link add netns ns2 name gtp0 type gtp role sgsn
  ip netns del ns1

Let's link the device to the socket's netns instead.

Now, gtp_net_exit_batch_rtnl() needs another netdev iteration to remove
all gtp devices in the netns.

[0]:
ref_tracker: net notrefcnt@000000003d6e7d05 has 1/2 users at
     sk_alloc (./include/net/net_namespace.h:345 net/core/sock.c:2236)
     inet_create (net/ipv4/af_inet.c:326 net/ipv4/af_inet.c:252)
     __sock_create (net/socket.c:1558)
     udp_sock_create4 (net/ipv4/udp_tunnel_core.c:18)
     gtp_create_sock (./include/net/udp_tunnel.h:59 drivers/net/gtp.c:1423)
     gtp_create_sockets (drivers/net/gtp.c:1447)
     gtp_newlink (drivers/net/gtp.c:1507)
     rtnl_newlink (net/core/rtnetlink.c:3786 net/core/rtnetlink.c:3897 net/core/rtnetlink.c:4012)
     rtnetlink_rcv_msg (net/core/rtnetlink.c:6922)
     netlink_rcv_skb (net/netlink/af_netlink.c:2542)
     netlink_unicast (net/netlink/af_netlink.c:1321 net/netlink/af_netlink.c:1347)
     netlink_sendmsg (net/netlink/af_netlink.c:1891)
     ____sys_sendmsg (net/socket.c:711 net/socket.c:726 net/socket.c:2583)
     ___sys_sendmsg (net/socket.c:2639)
     __sys_sendmsg (net/socket.c:2669)
     do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)

WARNING: CPU: 1 PID: 60 at lib/ref_tracker.c:179 ref_tracker_dir_exit (lib/ref_tracker.c:179)
Modules linked in:
CPU: 1 UID: 0 PID: 60 Comm: kworker/u16:2 Not tainted 6.13.0-rc5-00147-g4c1224501e9d #5
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
Workqueue: netns cleanup_net
RIP: 0010:ref_tracker_dir_exit (lib/ref_tracker.c:179)
Code: 00 00 00 fc ff df 4d 8b 26 49 bd 00 01 00 00 00 00 ad de 4c 39 f5 0f 85 df 00 00 00 48 8b 74 24 08 48 89 df e8 a5 cc 12 02 90 <0f> 0b 90 48 8d 6b 44 be 04 00 00 00 48 89 ef e8 80 de 67 ff 48 89
RSP: 0018:ff11000009a07b60 EFLAGS: 00010286
RAX: 0000000000002bd3 RBX: ff1100000f4e1aa0 RCX: 1ffffffff0e40ac6
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff8423ee3c
RBP: ff1100000f4e1af0 R08: 0000000000000001 R09: fffffbfff0e395ae
R10: 0000000000000001 R11: 0000000000036001 R12: ff1100000f4e1af0
R13: dead000000000100 R14: ff1100000f4e1af0 R15: dffffc0000000000
FS:  0000000000000000(0000) GS:ff1100006ce80000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00007f9b2464bd98 CR3: 0000000005286005 CR4: 0000000000771ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
 <TASK>
 ? __warn (kernel/panic.c:748)
 ? ref_tracker_dir_exit (lib/ref_tracker.c:179)
 ? report_bug (lib/bug.c:201 lib/bug.c:219)
 ? handle_bug (arch/x86/kernel/traps.c:285)
 ? exc_invalid_op (arch/x86/kernel/traps.c:309 (discriminator 1))
 ? asm_exc_invalid_op (./arch/x86/include/asm/idtentry.h:621)
 ? _raw_spin_unlock_irqrestore (./arch/x86/include/asm/irqflags.h:42 ./arch/x86/include/asm/irqflags.h:97 ./arch/x86/include/asm/irqflags.h:155 ./include/linux/spinlock_api_smp.h:151 kernel/locking/spinlock.c:194)
 ? ref_tracker_dir_exit (lib/ref_tracker.c:179)
 ? __pfx_ref_tracker_dir_exit (lib/ref_tracker.c:158)
 ? kfree (mm/slub.c:4613 mm/slub.c:4761)
 net_free (net/core/net_namespace.c:476 net/core/net_namespace.c:467)
 cleanup_net (net/core/net_namespace.c:664 (discriminator 3))
 process_one_work (kernel/workqueue.c:3229)
 worker_thread (kernel/workqueue.c:3304 kernel/workqueue.c:3391)
 kthread (kernel/kthread.c:389)
 ret_from_fork (arch/x86/kernel/process.c:147)
 ret_from_fork_asm (arch/x86/entry/entry_64.S:257)
 </TASK>

Fixes: 459aa660eb1d ("gtp: add initial driver for datapath of GPRS Tunneling Protocol (GTP-U)")
Reported-by: Xiao Liang <shaw.leon@gmail.com>
Closes: https://lore.kernel.org/netdev/20250104125732.17335-1-shaw.leon@gmail.com/
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
Cc: Pablo Neira Ayuso <pablo@netfilter.org>
Cc: Harald Welte <laforge@gnumonks.org>
---
 drivers/net/gtp.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 8e456d09d173..97fcd0c9525f 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -1524,7 +1524,7 @@ static int gtp_newlink(struct net *src_net, struct net_device *dev,
 		goto out_encap;
 	}
 
-	gn = net_generic(dev_net(dev), gtp_net_id);
+	gn = net_generic(src_net, gtp_net_id);
 	list_add(&gtp->list, &gn->gtp_dev_list);
 	dev->priv_destructor = gtp_destructor;
 
@@ -2480,6 +2480,11 @@ static void __net_exit gtp_net_exit_batch_rtnl(struct list_head *net_list,
 	list_for_each_entry(net, net_list, exit_list) {
 		struct gtp_net *gn = net_generic(net, gtp_net_id);
 		struct gtp_dev *gtp, *gtp_next;
+		struct net_device *dev;
+
+		for_each_netdev(net, dev)
+			if (dev->rtnl_link_ops == &gtp_link_ops)
+				gtp_dellink(dev, dev_to_kill);
 
 		list_for_each_entry_safe(gtp, gtp_next, &gn->gtp_dev_list, list)
 			gtp_dellink(gtp->dev, dev_to_kill);
-- 
2.39.5 (Apple Git-154)

[PATCH v1 net 3/3] pfcp: Destroy device along with udp socket's netns dismantle.

[Kuniyuki Iwashima]

pfcp_newlink() links the device to a list in dev_net(dev) instead
of net, where a udp tunnel socket is created.

Even when net is removed, the device stays alive on dev_net(dev).
Then, removing net triggers the splat below. [0]

In this example, pfcp0 is created in ns2, but the udp socket is
created in ns1.

  ip netns add ns1
  ip netns add ns2
  ip -n ns1 link add netns ns2 name pfcp0 type pfcp
  ip netns del ns1

Let's link the device to the socket's netns instead.

Now, pfcp_net_exit() needs another netdev iteration to remove
all pfcp devices in the netns.

pfcp_dev_list is not used under RCU, so the list API is converted
to the non-RCU variant.

pfcp_net_exit() can be converted to .exit_batch_rtnl() in net-next.

[0]:
ref_tracker: net notrefcnt@00000000128b34dc has 1/1 users at
     sk_alloc (./include/net/net_namespace.h:345 net/core/sock.c:2236)
     inet_create (net/ipv4/af_inet.c:326 net/ipv4/af_inet.c:252)
     __sock_create (net/socket.c:1558)
     udp_sock_create4 (net/ipv4/udp_tunnel_core.c:18)
     pfcp_create_sock (drivers/net/pfcp.c:168)
     pfcp_newlink (drivers/net/pfcp.c:182 drivers/net/pfcp.c:197)
     rtnl_newlink (net/core/rtnetlink.c:3786 net/core/rtnetlink.c:3897 net/core/rtnetlink.c:4012)
     rtnetlink_rcv_msg (net/core/rtnetlink.c:6922)
     netlink_rcv_skb (net/netlink/af_netlink.c:2542)
     netlink_unicast (net/netlink/af_netlink.c:1321 net/netlink/af_netlink.c:1347)
     netlink_sendmsg (net/netlink/af_netlink.c:1891)
     ____sys_sendmsg (net/socket.c:711 net/socket.c:726 net/socket.c:2583)
     ___sys_sendmsg (net/socket.c:2639)
     __sys_sendmsg (net/socket.c:2669)
     do_syscall_64 (arch/x86/entry/common.c:52 arch/x86/entry/common.c:83)
     entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:130)

WARNING: CPU: 1 PID: 11 at lib/ref_tracker.c:179 ref_tracker_dir_exit (lib/ref_tracker.c:179)
Modules linked in:
CPU: 1 UID: 0 PID: 11 Comm: kworker/u16:0 Not tainted 6.13.0-rc5-00147-g4c1224501e9d #5
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.16.0-0-gd239552ce722-prebuilt.qemu.org 04/01/2014
Workqueue: netns cleanup_net
RIP: 0010:ref_tracker_dir_exit (lib/ref_tracker.c:179)
Code: 00 00 00 fc ff df 4d 8b 26 49 bd 00 01 00 00 00 00 ad de 4c 39 f5 0f 85 df 00 00 00 48 8b 74 24 08 48 89 df e8 a5 cc 12 02 90 <0f> 0b 90 48 8d 6b 44 be 04 00 00 00 48 89 ef e8 80 de 67 ff 48 89
RSP: 0018:ff11000007f3fb60 EFLAGS: 00010286
RAX: 00000000000020ef RBX: ff1100000d6481e0 RCX: 1ffffffff0e40d82
RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffffffff8423ee3c
RBP: ff1100000d648230 R08: 0000000000000001 R09: fffffbfff0e395af
R10: 0000000000000001 R11: 0000000000000000 R12: ff1100000d648230
R13: dead000000000100 R14: ff1100000d648230 R15: dffffc0000000000
FS:  0000000000000000(0000) GS:ff1100006ce80000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
CR2: 00005620e1363990 CR3: 000000000eeb2002 CR4: 0000000000771ef0
DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7: 0000000000000400
PKRU: 55555554
Call Trace:
 <TASK>
 ? __warn (kernel/panic.c:748)
 ? ref_tracker_dir_exit (lib/ref_tracker.c:179)
 ? report_bug (lib/bug.c:201 lib/bug.c:219)
 ? handle_bug (arch/x86/kernel/traps.c:285)
 ? exc_invalid_op (arch/x86/kernel/traps.c:309 (discriminator 1))
 ? asm_exc_invalid_op (./arch/x86/include/asm/idtentry.h:621)
 ? _raw_spin_unlock_irqrestore (./arch/x86/include/asm/irqflags.h:42 ./arch/x86/include/asm/irqflags.h:97 ./arch/x86/include/asm/irqflags.h:155 ./include/linux/spinlock_api_smp.h:151 kernel/locking/spinlock.c:194)
 ? ref_tracker_dir_exit (lib/ref_tracker.c:179)
 ? __pfx_ref_tracker_dir_exit (lib/ref_tracker.c:158)
 ? kfree (mm/slub.c:4613 mm/slub.c:4761)
 net_free (net/core/net_namespace.c:476 net/core/net_namespace.c:467)
 cleanup_net (net/core/net_namespace.c:664 (discriminator 3))
 process_one_work (kernel/workqueue.c:3229)
 worker_thread (kernel/workqueue.c:3304 kernel/workqueue.c:3391)
 kthread (kernel/kthread.c:389)
 ret_from_fork (arch/x86/kernel/process.c:147)
 ret_from_fork_asm (arch/x86/entry/entry_64.S:257)
  </TASK>

Fixes: 76c8764ef36a ("pfcp: add PFCP module")
Reported-by: Xiao Liang <shaw.leon@gmail.com>
Closes: https://lore.kernel.org/netdev/20250104125732.17335-1-shaw.leon@gmail.com/
Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
---
Cc: Wojciech Drewek <wojciech.drewek@intel.com>
Cc: Marcin Szycik <marcin.szycik@linux.intel.com>
---
 drivers/net/pfcp.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/pfcp.c b/drivers/net/pfcp.c
index 69434fd13f96..68d0d9e92a22 100644
--- a/drivers/net/pfcp.c
+++ b/drivers/net/pfcp.c
@@ -206,8 +206,8 @@ static int pfcp_newlink(struct net *net, struct net_device *dev,
 		goto exit_del_pfcp_sock;
 	}
 
-	pn = net_generic(dev_net(dev), pfcp_net_id);
-	list_add_rcu(&pfcp->list, &pn->pfcp_dev_list);
+	pn = net_generic(net, pfcp_net_id);
+	list_add(&pfcp->list, &pn->pfcp_dev_list);
 
 	netdev_dbg(dev, "registered new PFCP interface\n");
 
@@ -224,7 +224,7 @@ static void pfcp_dellink(struct net_device *dev, struct list_head *head)
 {
 	struct pfcp_dev *pfcp = netdev_priv(dev);
 
-	list_del_rcu(&pfcp->list);
+	list_del(&pfcp->list);
 	unregister_netdevice_queue(dev, head);
 }
 
@@ -247,11 +247,16 @@ static int __net_init pfcp_net_init(struct net *net)
 static void __net_exit pfcp_net_exit(struct net *net)
 {
 	struct pfcp_net *pn = net_generic(net, pfcp_net_id);
-	struct pfcp_dev *pfcp;
+	struct pfcp_dev *pfcp, *pfcp_next;
+	struct net_device *dev;
 	LIST_HEAD(list);
 
 	rtnl_lock();
-	list_for_each_entry(pfcp, &pn->pfcp_dev_list, list)
+	for_each_netdev(net, dev)
+		if (dev->rtnl_link_ops == &pfcp_link_ops)
+			pfcp_dellink(dev, &list);
+
+	list_for_each_entry_safe(pfcp, pfcp_next, &pn->pfcp_dev_list, list)
 		pfcp_dellink(pfcp->dev, &list);
 
 	unregister_netdevice_many(&list);
-- 
2.39.5 (Apple Git-154)

Re: [PATCH v1 net 2/3] gtp: Destroy device along with udp socket's netns dismantle.

[Xiao Liang]

On Wed, Jan 8, 2025 at 2:29 PM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
[...]
> @@ -2480,6 +2480,11 @@ static void __net_exit gtp_net_exit_batch_rtnl(struct list_head *net_list,
>         list_for_each_entry(net, net_list, exit_list) {
>                 struct gtp_net *gn = net_generic(net, gtp_net_id);
>                 struct gtp_dev *gtp, *gtp_next;
> +               struct net_device *dev;
> +
> +               for_each_netdev(net, dev)
> +                       if (dev->rtnl_link_ops == &gtp_link_ops)
> +                               gtp_dellink(dev, dev_to_kill);

I'm not sure, but do we usually have to clean up devices
in the netns being deleted? Seems default_device_ops
can handle it.

>
>                 list_for_each_entry_safe(gtp, gtp_next, &gn->gtp_dev_list, list)
>                         gtp_dellink(gtp->dev, dev_to_kill);
> --
> 2.39.5 (Apple Git-154)
>

Re: [PATCH v1 net 2/3] gtp: Destroy device along with udp socket's netns dismantle.

[Kuniyuki Iwashima]

From: Xiao Liang <shaw.leon@gmail.com>
Date: Wed, 8 Jan 2025 16:55:16 +0800
> On Wed, Jan 8, 2025 at 2:29 PM Kuniyuki Iwashima <kuniyu@amazon.com> wrote:
> [...]
> > @@ -2480,6 +2480,11 @@ static void __net_exit gtp_net_exit_batch_rtnl(struct list_head *net_list,
> >         list_for_each_entry(net, net_list, exit_list) {
> >                 struct gtp_net *gn = net_generic(net, gtp_net_id);
> >                 struct gtp_dev *gtp, *gtp_next;
> > +               struct net_device *dev;
> > +
> > +               for_each_netdev(net, dev)
> > +                       if (dev->rtnl_link_ops == &gtp_link_ops)
> > +                               gtp_dellink(dev, dev_to_kill);
> 
> I'm not sure, but do we usually have to clean up devices
> in the netns being deleted? Seems default_device_ops
> can handle it.

default_device_ops handles netns dismantle, but we still
need this because the module is unloadable.

I have a patch that moves default_device_ops stuff after
some ->exit_batch_rtnl() to save one RTNL dance, which will
make it clear when default_device_ops is executed.

Re: [PATCH v1 net 1/3] gtp: Use for_each_netdev_rcu() in gtp_genl_dump_pdp().

[Simon Horman]

On Wed, Jan 08, 2025 at 03:28:32PM +0900, Kuniyuki Iwashima wrote:
> gtp_newlink() links the gtp device to a list in dev_net(dev).
> 
> However, even after the gtp device is moved to another netns,
> it stays on the list but should be invisible.
> 
> Let's use for_each_netdev_rcu() for netdev traversal in
> gtp_genl_dump_pdp().
> 
> Note that gtp_dev_list is no longer used under RCU, so list
> helpers are converted to the non-RCU variant.
> 
> Fixes: 459aa660eb1d ("gtp: add initial driver for datapath of GPRS Tunneling Protocol (GTP-U)")
> Reported-by: Xiao Liang <shaw.leon@gmail.com>
> Closes: https://lore.kernel.org/netdev/CABAhCOQdBL6h9M2C+kd+bGivRJ9Q72JUxW+-gur0nub_=PmFPA@mail.gmail.com/
> Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> ---
> Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> Cc: Harald Welte <laforge@gnumonks.org>

...

> @@ -2280,7 +2281,10 @@ static int gtp_genl_dump_pdp(struct sk_buff *skb,
>  		return 0;
>  
>  	rcu_read_lock();
> -	list_for_each_entry_rcu(gtp, &gn->gtp_dev_list, list) {
> +	for_each_netdev_rcu(net, dev) {
> +		if (dev->rtnl_link_ops != &gtp_link_ops)
> +			continue;
> +
>  		if (last_gtp && last_gtp != gtp)
>  			continue;
>  		else

Hi Iwashima-san,

With this change gtp seems to be uninitialised here.
And, also, it looks like gn is now set but otherwise unused in this function.

Flagged by W=1 builds with clang-19.

...

Re: [PATCH v1 net 1/3] gtp: Use for_each_netdev_rcu() in gtp_genl_dump_pdp().

[Kuniyuki Iwashima]

From: Simon Horman <horms@kernel.org>
Date: Wed, 8 Jan 2025 10:40:40 +0000
> On Wed, Jan 08, 2025 at 03:28:32PM +0900, Kuniyuki Iwashima wrote:
> > gtp_newlink() links the gtp device to a list in dev_net(dev).
> > 
> > However, even after the gtp device is moved to another netns,
> > it stays on the list but should be invisible.
> > 
> > Let's use for_each_netdev_rcu() for netdev traversal in
> > gtp_genl_dump_pdp().
> > 
> > Note that gtp_dev_list is no longer used under RCU, so list
> > helpers are converted to the non-RCU variant.
> > 
> > Fixes: 459aa660eb1d ("gtp: add initial driver for datapath of GPRS Tunneling Protocol (GTP-U)")
> > Reported-by: Xiao Liang <shaw.leon@gmail.com>
> > Closes: https://lore.kernel.org/netdev/CABAhCOQdBL6h9M2C+kd+bGivRJ9Q72JUxW+-gur0nub_=PmFPA@mail.gmail.com/
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@amazon.com>
> > ---
> > Cc: Pablo Neira Ayuso <pablo@netfilter.org>
> > Cc: Harald Welte <laforge@gnumonks.org>
> 
> ...
> 
> > @@ -2280,7 +2281,10 @@ static int gtp_genl_dump_pdp(struct sk_buff *skb,
> >  		return 0;
> >  
> >  	rcu_read_lock();
> > -	list_for_each_entry_rcu(gtp, &gn->gtp_dev_list, list) {
> > +	for_each_netdev_rcu(net, dev) {
> > +		if (dev->rtnl_link_ops != &gtp_link_ops)
> > +			continue;
> > +
> >  		if (last_gtp && last_gtp != gtp)
> >  			continue;
> >  		else
> 
> Hi Iwashima-san,
> 
> With this change gtp seems to be uninitialised here.
> And, also, it looks like gn is now set but otherwise unused in this function.
> 
> Flagged by W=1 builds with clang-19.

Ah, thank you for catching these !

I'll squash this to v2.

---8<---
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index b905b7bc46cb..0d03e150efc6 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -2298,9 +2298,6 @@ static int gtp_genl_dump_pdp(struct sk_buff *skb,
 	struct net *net = sock_net(skb->sk);
 	struct net_device *dev;
 	struct pdp_ctx *pctx;
-	struct gtp_net *gn;
-
-	gn = net_generic(net, gtp_net_id);
 
 	if (cb->args[4])
 		return 0;
@@ -2310,6 +2307,8 @@ static int gtp_genl_dump_pdp(struct sk_buff *skb,
 		if (dev->rtnl_link_ops != &gtp_link_ops)
 			continue;
 
+		gtp = netdev_priv(dev);
+
 		if (last_gtp && last_gtp != gtp)
 			continue;
 		else
---8<---

Re: [PATCH v1 net 1/3] gtp: Use for_each_netdev_rcu() in gtp_genl_dump_pdp().

[kernel test robot]

Hi Kuniyuki,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Kuniyuki-Iwashima/gtp-Use-for_each_netdev_rcu-in-gtp_genl_dump_pdp/20250108-143107
base:   net/main
patch link:    https://lore.kernel.org/r/20250108062834.11117-2-kuniyu%40amazon.com
patch subject: [PATCH v1 net 1/3] gtp: Use for_each_netdev_rcu() in gtp_genl_dump_pdp().
config: powerpc-randconfig-r073-20250109 (https://download.01.org/0day-ci/archive/20250109/202501090934.2wvM0EAi-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 096551537b2a747a3387726ca618ceeb3950e9bc)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250109/202501090934.2wvM0EAi-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501090934.2wvM0EAi-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/net/gtp.c:2276:18: warning: variable 'gn' set but not used [-Wunused-but-set-variable]
    2276 |         struct gtp_net *gn;
         |                         ^
   drivers/net/gtp.c:2288:31: warning: variable 'gtp' is uninitialized when used here [-Wuninitialized]
    2288 |                 if (last_gtp && last_gtp != gtp)
         |                                             ^~~
   drivers/net/gtp.c:2271:64: note: initialize the variable 'gtp' to silence this warning
    2271 |         struct gtp_dev *last_gtp = (struct gtp_dev *)cb->args[2], *gtp;
         |                                                                       ^
         |                                                                        = NULL
   2 warnings generated.


vim +/gn +2276 drivers/net/gtp.c

459aa660eb1d8c Pablo Neira        2016-05-09  2267  
459aa660eb1d8c Pablo Neira        2016-05-09  2268  static int gtp_genl_dump_pdp(struct sk_buff *skb,
459aa660eb1d8c Pablo Neira        2016-05-09  2269  				struct netlink_callback *cb)
459aa660eb1d8c Pablo Neira        2016-05-09  2270  {
459aa660eb1d8c Pablo Neira        2016-05-09  2271  	struct gtp_dev *last_gtp = (struct gtp_dev *)cb->args[2], *gtp;
94a6d9fb88df43 Taehee Yoo         2019-12-11  2272  	int i, j, bucket = cb->args[0], skip = cb->args[1];
459aa660eb1d8c Pablo Neira        2016-05-09  2273  	struct net *net = sock_net(skb->sk);
766a8e9a311068 Kuniyuki Iwashima  2025-01-08  2274  	struct net_device *dev;
459aa660eb1d8c Pablo Neira        2016-05-09  2275  	struct pdp_ctx *pctx;
94a6d9fb88df43 Taehee Yoo         2019-12-11 @2276  	struct gtp_net *gn;
94a6d9fb88df43 Taehee Yoo         2019-12-11  2277  
94a6d9fb88df43 Taehee Yoo         2019-12-11  2278  	gn = net_generic(net, gtp_net_id);
459aa660eb1d8c Pablo Neira        2016-05-09  2279  
459aa660eb1d8c Pablo Neira        2016-05-09  2280  	if (cb->args[4])
459aa660eb1d8c Pablo Neira        2016-05-09  2281  		return 0;
459aa660eb1d8c Pablo Neira        2016-05-09  2282  
94a6d9fb88df43 Taehee Yoo         2019-12-11  2283  	rcu_read_lock();
766a8e9a311068 Kuniyuki Iwashima  2025-01-08  2284  	for_each_netdev_rcu(net, dev) {
766a8e9a311068 Kuniyuki Iwashima  2025-01-08  2285  		if (dev->rtnl_link_ops != &gtp_link_ops)
766a8e9a311068 Kuniyuki Iwashima  2025-01-08  2286  			continue;
766a8e9a311068 Kuniyuki Iwashima  2025-01-08  2287  
459aa660eb1d8c Pablo Neira        2016-05-09  2288  		if (last_gtp && last_gtp != gtp)
459aa660eb1d8c Pablo Neira        2016-05-09  2289  			continue;
459aa660eb1d8c Pablo Neira        2016-05-09  2290  		else
459aa660eb1d8c Pablo Neira        2016-05-09  2291  			last_gtp = NULL;
459aa660eb1d8c Pablo Neira        2016-05-09  2292  
94a6d9fb88df43 Taehee Yoo         2019-12-11  2293  		for (i = bucket; i < gtp->hash_size; i++) {
94a6d9fb88df43 Taehee Yoo         2019-12-11  2294  			j = 0;
94a6d9fb88df43 Taehee Yoo         2019-12-11  2295  			hlist_for_each_entry_rcu(pctx, &gtp->tid_hash[i],
94a6d9fb88df43 Taehee Yoo         2019-12-11  2296  						 hlist_tid) {
94a6d9fb88df43 Taehee Yoo         2019-12-11  2297  				if (j >= skip &&
94a6d9fb88df43 Taehee Yoo         2019-12-11  2298  				    gtp_genl_fill_info(skb,
459aa660eb1d8c Pablo Neira        2016-05-09  2299  					    NETLINK_CB(cb->skb).portid,
459aa660eb1d8c Pablo Neira        2016-05-09  2300  					    cb->nlh->nlmsg_seq,
846c68f7f1ac82 Yoshiyuki Kurauchi 2020-04-30  2301  					    NLM_F_MULTI,
94a6d9fb88df43 Taehee Yoo         2019-12-11  2302  					    cb->nlh->nlmsg_type, pctx)) {
459aa660eb1d8c Pablo Neira        2016-05-09  2303  					cb->args[0] = i;
94a6d9fb88df43 Taehee Yoo         2019-12-11  2304  					cb->args[1] = j;
459aa660eb1d8c Pablo Neira        2016-05-09  2305  					cb->args[2] = (unsigned long)gtp;
459aa660eb1d8c Pablo Neira        2016-05-09  2306  					goto out;
459aa660eb1d8c Pablo Neira        2016-05-09  2307  				}
94a6d9fb88df43 Taehee Yoo         2019-12-11  2308  				j++;
459aa660eb1d8c Pablo Neira        2016-05-09  2309  			}
94a6d9fb88df43 Taehee Yoo         2019-12-11  2310  			skip = 0;
459aa660eb1d8c Pablo Neira        2016-05-09  2311  		}
94a6d9fb88df43 Taehee Yoo         2019-12-11  2312  		bucket = 0;
459aa660eb1d8c Pablo Neira        2016-05-09  2313  	}
459aa660eb1d8c Pablo Neira        2016-05-09  2314  	cb->args[4] = 1;
459aa660eb1d8c Pablo Neira        2016-05-09  2315  out:
94a6d9fb88df43 Taehee Yoo         2019-12-11  2316  	rcu_read_unlock();
459aa660eb1d8c Pablo Neira        2016-05-09  2317  	return skb->len;
459aa660eb1d8c Pablo Neira        2016-05-09  2318  }
459aa660eb1d8c Pablo Neira        2016-05-09  2319  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v1 net 1/3] gtp: Use for_each_netdev_rcu() in gtp_genl_dump_pdp().

[kernel test robot]

Hi Kuniyuki,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Kuniyuki-Iwashima/gtp-Use-for_each_netdev_rcu-in-gtp_genl_dump_pdp/20250108-143107
base:   net/main
patch link:    https://lore.kernel.org/r/20250108062834.11117-2-kuniyu%40amazon.com
patch subject: [PATCH v1 net 1/3] gtp: Use for_each_netdev_rcu() in gtp_genl_dump_pdp().
config: arm64-allmodconfig (https://download.01.org/0day-ci/archive/20250110/202501101052.pULMpd2R-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250110/202501101052.pULMpd2R-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501101052.pULMpd2R-lkp@intel.com/

All warnings (new ones prefixed by >>):

   drivers/net/gtp.c:2276:18: warning: variable 'gn' set but not used [-Wunused-but-set-variable]
    2276 |         struct gtp_net *gn;
         |                         ^
>> drivers/net/gtp.c:2288:31: warning: variable 'gtp' is uninitialized when used here [-Wuninitialized]
    2288 |                 if (last_gtp && last_gtp != gtp)
         |                                             ^~~
   drivers/net/gtp.c:2271:64: note: initialize the variable 'gtp' to silence this warning
    2271 |         struct gtp_dev *last_gtp = (struct gtp_dev *)cb->args[2], *gtp;
         |                                                                       ^
         |                                                                        = NULL
   2 warnings generated.


vim +/gtp +2288 drivers/net/gtp.c

459aa660eb1d8c Pablo Neira        2016-05-09  2267  
459aa660eb1d8c Pablo Neira        2016-05-09  2268  static int gtp_genl_dump_pdp(struct sk_buff *skb,
459aa660eb1d8c Pablo Neira        2016-05-09  2269  				struct netlink_callback *cb)
459aa660eb1d8c Pablo Neira        2016-05-09  2270  {
459aa660eb1d8c Pablo Neira        2016-05-09  2271  	struct gtp_dev *last_gtp = (struct gtp_dev *)cb->args[2], *gtp;
94a6d9fb88df43 Taehee Yoo         2019-12-11  2272  	int i, j, bucket = cb->args[0], skip = cb->args[1];
459aa660eb1d8c Pablo Neira        2016-05-09  2273  	struct net *net = sock_net(skb->sk);
766a8e9a311068 Kuniyuki Iwashima  2025-01-08  2274  	struct net_device *dev;
459aa660eb1d8c Pablo Neira        2016-05-09  2275  	struct pdp_ctx *pctx;
94a6d9fb88df43 Taehee Yoo         2019-12-11  2276  	struct gtp_net *gn;
94a6d9fb88df43 Taehee Yoo         2019-12-11  2277  
94a6d9fb88df43 Taehee Yoo         2019-12-11  2278  	gn = net_generic(net, gtp_net_id);
459aa660eb1d8c Pablo Neira        2016-05-09  2279  
459aa660eb1d8c Pablo Neira        2016-05-09  2280  	if (cb->args[4])
459aa660eb1d8c Pablo Neira        2016-05-09  2281  		return 0;
459aa660eb1d8c Pablo Neira        2016-05-09  2282  
94a6d9fb88df43 Taehee Yoo         2019-12-11  2283  	rcu_read_lock();
766a8e9a311068 Kuniyuki Iwashima  2025-01-08  2284  	for_each_netdev_rcu(net, dev) {
766a8e9a311068 Kuniyuki Iwashima  2025-01-08  2285  		if (dev->rtnl_link_ops != &gtp_link_ops)
766a8e9a311068 Kuniyuki Iwashima  2025-01-08  2286  			continue;
766a8e9a311068 Kuniyuki Iwashima  2025-01-08  2287  
459aa660eb1d8c Pablo Neira        2016-05-09 @2288  		if (last_gtp && last_gtp != gtp)
459aa660eb1d8c Pablo Neira        2016-05-09  2289  			continue;
459aa660eb1d8c Pablo Neira        2016-05-09  2290  		else
459aa660eb1d8c Pablo Neira        2016-05-09  2291  			last_gtp = NULL;
459aa660eb1d8c Pablo Neira        2016-05-09  2292  
94a6d9fb88df43 Taehee Yoo         2019-12-11  2293  		for (i = bucket; i < gtp->hash_size; i++) {
94a6d9fb88df43 Taehee Yoo         2019-12-11  2294  			j = 0;
94a6d9fb88df43 Taehee Yoo         2019-12-11  2295  			hlist_for_each_entry_rcu(pctx, &gtp->tid_hash[i],
94a6d9fb88df43 Taehee Yoo         2019-12-11  2296  						 hlist_tid) {
94a6d9fb88df43 Taehee Yoo         2019-12-11  2297  				if (j >= skip &&
94a6d9fb88df43 Taehee Yoo         2019-12-11  2298  				    gtp_genl_fill_info(skb,
459aa660eb1d8c Pablo Neira        2016-05-09  2299  					    NETLINK_CB(cb->skb).portid,
459aa660eb1d8c Pablo Neira        2016-05-09  2300  					    cb->nlh->nlmsg_seq,
846c68f7f1ac82 Yoshiyuki Kurauchi 2020-04-30  2301  					    NLM_F_MULTI,
94a6d9fb88df43 Taehee Yoo         2019-12-11  2302  					    cb->nlh->nlmsg_type, pctx)) {
459aa660eb1d8c Pablo Neira        2016-05-09  2303  					cb->args[0] = i;
94a6d9fb88df43 Taehee Yoo         2019-12-11  2304  					cb->args[1] = j;
459aa660eb1d8c Pablo Neira        2016-05-09  2305  					cb->args[2] = (unsigned long)gtp;
459aa660eb1d8c Pablo Neira        2016-05-09  2306  					goto out;
459aa660eb1d8c Pablo Neira        2016-05-09  2307  				}
94a6d9fb88df43 Taehee Yoo         2019-12-11  2308  				j++;
459aa660eb1d8c Pablo Neira        2016-05-09  2309  			}
94a6d9fb88df43 Taehee Yoo         2019-12-11  2310  			skip = 0;
459aa660eb1d8c Pablo Neira        2016-05-09  2311  		}
94a6d9fb88df43 Taehee Yoo         2019-12-11  2312  		bucket = 0;
459aa660eb1d8c Pablo Neira        2016-05-09  2313  	}
459aa660eb1d8c Pablo Neira        2016-05-09  2314  	cb->args[4] = 1;
459aa660eb1d8c Pablo Neira        2016-05-09  2315  out:
94a6d9fb88df43 Taehee Yoo         2019-12-11  2316  	rcu_read_unlock();
459aa660eb1d8c Pablo Neira        2016-05-09  2317  	return skb->len;
459aa660eb1d8c Pablo Neira        2016-05-09  2318  }
459aa660eb1d8c Pablo Neira        2016-05-09  2319  

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki