* [PATCH net v2] gtp: add synchronize_net() in gtp_newlink() error path to prevent use-after-free
@ 2026-08-20 2:07 Cen Zhang (Microsoft)
2026-08-21 13:53 ` Simon Horman
0 siblings, 1 reply; 2+ messages in thread
From: Cen Zhang (Microsoft) @ 2026-08-20 2:07 UTC (permalink / raw)
To: pablo, laforge, andrew+netdev, davem, edumazet, kuba, pabeni
Cc: osmocom-net-gprs, netdev, linux-kernel, AutonomousCodeSecurity,
xmei5, tgopinath, kys, blbllhy
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: AutonomousCodeSecurity@microsoft.com
Reported-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
Reported-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
---
v2: Add the net tree subject prefix.
drivers/net/gtp.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 9a12cc53da00..f11bcc0c0ac4 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -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;
--
2.52.0
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH net v2] gtp: add synchronize_net() in gtp_newlink() error path to prevent use-after-free
2026-08-20 2:07 [PATCH net v2] gtp: add synchronize_net() in gtp_newlink() error path to prevent use-after-free Cen Zhang (Microsoft)
@ 2026-08-21 13:53 ` Simon Horman
0 siblings, 0 replies; 2+ messages in thread
From: Simon Horman @ 2026-08-21 13:53 UTC (permalink / raw)
To: Cen Zhang (Microsoft)
Cc: pablo, laforge, andrew+netdev, davem, edumazet, kuba, pabeni,
osmocom-net-gprs, netdev, linux-kernel, AutonomousCodeSecurity,
xmei5, tgopinath, kys
On Wed, Aug 19, 2026 at 10:07:35PM -0400, Cen Zhang (Microsoft) 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.
>
> 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: AutonomousCodeSecurity@microsoft.com
> Reported-by: Xiang Mei (Microsoft) <xmei5@asu.edu>
> Reported-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
> Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
> ---
> v2: Add the net tree subject prefix.
>
> drivers/net/gtp.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
> index 9a12cc53da00..f11bcc0c0ac4 100644
> --- a/drivers/net/gtp.c
> +++ b/drivers/net/gtp.c
> @@ -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.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-08-21 13:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-20 2:07 [PATCH net v2] gtp: add synchronize_net() in gtp_newlink() error path to prevent use-after-free Cen Zhang (Microsoft)
2026-08-21 13:53 ` Simon Horman
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox