Re: [PATCH net v2] gtp: add synchronize_net() in gtp_newlink() error path to prevent use-after-free
From: Simon Horman <horms@kernel.org>
Date: 2026-08-21 13:53:15
Also in:
lkml
On Wed, Aug 19, 2026 at 10:07:35PM -0400, Cen Zhang (Microsoft) wrote:
quoted hunk ↗ jump to hunk
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) <redacted> Reported-by: Cen Zhang (Microsoft) <redacted> Signed-off-by: Cen Zhang (Microsoft) <redacted> --- 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.