Re: [PATCH net v2] gtp: add synchronize_net() in gtp_newlink() error path to prevent use-after-free

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: [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(+)
> 
> 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.