Skip to contentSkip to conversation

gtp: add synchronize_net() in gtp_newlink() error path to prevent use-after-free

PATCH Applied Cen Zhang (Microsoft) 3 participants 3 messages Aug 20 – Aug 24

Subsystems (3)
Networking Network drivers GTP (GPRS Tunneling Protocol)
lore
Share & tools
lore

Cen Zhang (Microsoft)

author
#0 lore raw

PATCH v2 1/1 gtp: add synchronize_net() in gtp_newlink() error path to prevent use-after-free

gtp_newlink()'s error path frees tid_hash and addr_hash without waiting for an RCU grace period after clearing sk_user_data. A concurrent gtp_encap_recv() in softirq may still hold the gtp_dev pointer obtained via rcu_dereference_sk_user_data() and access the freed memory.

  BUG: KASAN: slab-use-after-free in gtp0_pdp_find+0x1f6/0x200 (gtp.c:152)
  Call Trace:
   <IRQ>
   gtp0_pdp_find+0x1f6/0x200
   gtp_encap_recv+0x527/0x24b0
   udp_queue_rcv_one_skb+0x75f/0xc10

Add synchronize_net() before the kfree calls in out_hashtable, which covers all error paths from both gtp_encap_enable() and gtp_create_sockets().

Fixes 459aa660eb1d8ce6 ("gtp: add initial driver for datapath of GPRS Tunneling Protocol (GTP-U)")
Reported-by [email protected]
Reported-by Xiang Mei (Microsoft) <[email protected]>
Reported-by Cen Zhang (Microsoft) <[email protected]>
Signed-off-by Cen Zhang (Microsoft) <[email protected]>

---
v2: Add the net tree subject prefix.

 drivers/net/gtp.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/net/gtp.c +2 -0
index 9a12cc53da00..f11bcc0c0ac4 100644
@@ -1543,6 +1543,8 @@ static int gtp_newlink(struct net_device *dev,
 out_encap:
 	gtp_encap_disable(gtp);
 out_hashtable:
+	/* Wait for RCU readers that may still reference this gtp_dev. */
+	synchronize_net();
 	kfree(gtp->addr_hash);
 	kfree(gtp->tid_hash);
 	return err;
Signature · 1 line
2.52.0
reviewer
#1 lore raw
Quoted 33 lines · Cen Zhang (Microsoft)

gtp_newlink()'s error path frees tid_hash and addr_hash without waiting for an RCU grace period after clearing sk_user_data. A concurrent gtp_encap_recv() in softirq may still hold the gtp_dev pointer obtained via rcu_dereference_sk_user_data() and access the freed memory.

  BUG: KASAN: slab-use-after-free in gtp0_pdp_find+0x1f6/0x200 (gtp.c:152)
  Call Trace:
   <IRQ>
   gtp0_pdp_find+0x1f6/0x200
   gtp_encap_recv+0x527/0x24b0
   udp_queue_rcv_one_skb+0x75f/0xc10

Add synchronize_net() before the kfree calls in out_hashtable, which covers all error paths from both gtp_encap_enable() and gtp_create_sockets().

Fixes 459aa660eb1d8ce6 ("gtp: add initial driver for datapath of GPRS Tunneling Protocol (GTP-U)")
Reported-by [email protected]
Reported-by Xiang Mei (Microsoft) <[email protected]>
Reported-by Cen Zhang (Microsoft) <[email protected]>
Signed-off-by Cen Zhang (Microsoft) <[email protected]>

---
v2: Add the net tree subject prefix.

 drivers/net/gtp.c | 2 ++
 1 file changed, 2 insertions(+)
drivers/net/gtp.c +0 -0
index 9a12cc53da00..f11bcc0c0ac4 100644

drivers/net/gtp.c static int gtp_newlink(struct net_device *dev, · line 1550

Quoted 8 lines
drivers/net/gtp.c +2 -0
@@ -1543,6 +1543,8 @@ static int gtp_newlink(struct net_device *dev,
 out_encap:
 	gtp_encap_disable(gtp);
 out_hashtable:
+	/* Wait for RCU readers that may still reference this gtp_dev. */
+	synchronize_net();
 	kfree(gtp->addr_hash);
 	kfree(gtp->tid_hash);
 	return err;

A few thoughts on this, because in my opinion synchronize_net() is a heavy operation.

1. I think it's good to have a simple fix for net. But I do wonder if there is some value in making the call to synchronize_net() conditional. Because it seem to me that there are failure modes where out_hashtable is reached but synchronisation is not required.

2. In the longer run, I wonder if it would be worth looking into re-ordering initialisation so the problem described cannot occur.

CI Applied
#2 lore raw
In reply to Cen Zhang (Microsoft)
This patch was applied to netdev/net.git (main) by Jakub Kicinski <[email protected]>:

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <[email protected]>:

Quoted 6 lines · On Wed, 19 Aug 2026 22:07:35 -0400 you wrote:

gtp_newlink()'s error path frees tid_hash and addr_hash without waiting for an RCU grace period after clearing sk_user_data. A concurrent gtp_encap_recv() in softirq may still hold the gtp_dev pointer obtained via rcu_dereference_sk_user_data() and access the freed memory.

Quoted 8 lines
  BUG: KASAN: slab-use-after-free in gtp0_pdp_find+0x1f6/0x200 (gtp.c:152)
  Call Trace:
   <IRQ>
   gtp0_pdp_find+0x1f6/0x200
   gtp_encap_recv+0x527/0x24b0
   udp_queue_rcv_one_skb+0x75f/0xc10

[...]

Here is the summary with links:
- [net,v2] gtp: add synchronize_net() in gtp_newlink() error path to prevent use-after-free

    https://git.kernel.org/netdev/net/c/d989e22ae980

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html