[PATCH net v2 0/2] gtp: fix sk_created race conditions

[PATCH net v2 0/2] gtp: fix sk_created race conditions

[Cen Zhang (Microsoft)]

Two races exist around the sk_created flag in the GTP driver: an
initialization ordering issue where sk_created is published before the
socket pointers are assigned, and a teardown lifetime issue where
concurrent readers can still use the sockets after they are freed. They
are split into separate patches because they require fundamentally
different fixes -- memory barriers for ordering vs synchronize_net() and
RTNL for lifetime.

Changes since v1 [1]:
- Use smp_store_release()/smp_load_acquire() for proper memory ordering
  as suggested by Simon Horman.
- Add patch 2/2 to fix teardown races in gtp_encap_disable(): reorder
  to clear sk_created first, add synchronize_net() for softirq paths,
  and hold RTNL in gtp_genl_send_echo_req() for the process-context
  path that synchronize_net() cannot protect.

Patch 1/2 fixes the initialization ordering race.
Patch 2/2 fixes the teardown lifetime races (softirq + genl).

[1] https://lore.kernel.org/netdev/20260816035205.57966-1-blbllhy@gmail.com/

Cen Zhang (Microsoft) (2):
  gtp: fix sk_created publication race in gtp_create_sockets()
  gtp: fix use-after-free during GTP device teardown

 drivers/net/gtp.c | 70 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 51 insertions(+), 19 deletions(-)


base-commit: b70b7b71561f2be1183b6046964d950c2b203bfc
-- 
2.55.0

[PATCH net v2 1/2] gtp: fix sk_created publication race in gtp_create_sockets()

[Cen Zhang (Microsoft)]

In gtp_create_sockets(), gtp->sk_created is set to true before gtp->sk0
and gtp->sk1u are assigned. Without memory ordering guarantees, a
concurrent GTP Echo packet on another CPU can observe sk_created == true
while gtp->sk0 is still NULL, causing a kernel panic.

  KASAN: null-ptr-deref in range [0x0000000000000010-0x0000000000000017]
  RIP: 0010:gtp_encap_recv  (drivers/net/gtp.c:542 gtp0_handle_echo_resp)
  Call Trace:
   <IRQ>
   udp_queue_rcv_one_skb
   ip_protocol_deliver_rcu
   ip_local_deliver
  Kernel panic - not syncing: Fatal exception in interrupt

Use smp_store_release() when publishing sk_created and
smp_load_acquire() on every lockless read. This ensures that all prior
stores (sk0, sk1u assignments) are visible before any reader can observe
the flag as true, on all architectures. Annotate all remaining lockless
sk_created accesses with smp_store_release()/smp_load_acquire().

Suggested-by: Simon Horman <horms@kernel.org>
Fixes: b20dc3c68458 ("gtp: Allow to create GTP device without FDs")
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: Use smp_store_release()/smp_load_acquire() to provide proper memory
    ordering as suggested by Simon Horman.
v1: https://lore.kernel.org/netdev/20260816035205.57966-1-blbllhy@gmail.com/
---
 drivers/net/gtp.c | 29 ++++++++++++++++++++++-------
 1 file changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index 298efc76a56b..ead519ee18d1 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -603,10 +603,12 @@ static int gtp0_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
 	 * there is no daemon running in userspace which would
 	 * handle echo request.
 	 */
-	if (gtp0->type == GTP_ECHO_REQ && gtp->sk_created)
+	/* Pairs with smp_store_release() in gtp_create_sockets(). */
+	if (gtp0->type == GTP_ECHO_REQ && smp_load_acquire(&gtp->sk_created))
 		return gtp0_send_echo_resp(gtp, skb);
 
-	if (gtp0->type == GTP_ECHO_RSP && gtp->sk_created)
+	/* Pairs with smp_store_release() in gtp_create_sockets(). */
+	if (gtp0->type == GTP_ECHO_RSP && smp_load_acquire(&gtp->sk_created))
 		return gtp0_handle_echo_resp(gtp, skb);
 
 	if (gtp0->type != GTP_TPDU)
@@ -811,10 +813,12 @@ static int gtp1u_udp_encap_recv(struct gtp_dev *gtp, struct sk_buff *skb)
 	 * there is no daemon running in userspace which would
 	 * handle echo request.
 	 */
-	if (gtp1->type == GTP_ECHO_REQ && gtp->sk_created)
+	/* Pairs with smp_store_release() in gtp_create_sockets(). */
+	if (gtp1->type == GTP_ECHO_REQ && smp_load_acquire(&gtp->sk_created))
 		return gtp1u_send_echo_resp(gtp, skb);
 
-	if (gtp1->type == GTP_ECHO_RSP && gtp->sk_created)
+	/* Pairs with smp_store_release() in gtp_create_sockets(). */
+	if (gtp1->type == GTP_ECHO_RSP && smp_load_acquire(&gtp->sk_created))
 		return gtp1u_handle_echo_resp(gtp, skb);
 
 	if (gtp1->type != GTP_TPDU)
@@ -894,7 +898,10 @@ static void gtp_encap_disable(struct gtp_dev *gtp)
 	if (gtp->sk_created) {
 		udp_tunnel_sock_release(gtp->sk0);
 		udp_tunnel_sock_release(gtp->sk1u);
-		gtp->sk_created = false;
+		/* Pairs with smp_load_acquire() in the RX and
+		 * genl echo paths.
+		 */
+		smp_store_release(&gtp->sk_created, false);
 		gtp->sk0 = NULL;
 		gtp->sk1u = NULL;
 	} else {
@@ -1462,10 +1469,15 @@ static int gtp_create_sockets(struct gtp_dev *gtp, const struct nlattr *nla,
 		return PTR_ERR(sk1u);
 	}
 
-	gtp->sk_created = true;
 	gtp->sk0 = sk0;
 	gtp->sk1u = sk1u;
 
+	/* Ensure sk0/sk1u are visible before sk_created is set.
+	 * Pairs with smp_load_acquire() in the RX and genl
+	 * echo paths.
+	 */
+	smp_store_release(&gtp->sk_created, true);
+
 	return 0;
 }
 
@@ -2365,7 +2377,10 @@ static int gtp_genl_send_echo_req(struct sk_buff *skb, struct genl_info *info)
 	if (!gtp)
 		return -ENODEV;
 
-	if (!gtp->sk_created)
+	/* Pairs with smp_store_release() in gtp_create_sockets()
+	 * and gtp_encap_disable().
+	 */
+	if (!smp_load_acquire(&gtp->sk_created))
 		return -EOPNOTSUPP;
 	if (!(gtp->dev->flags & IFF_UP))
 		return -ENETDOWN;
-- 
2.55.0

[PATCH net v2 2/2] gtp: fix use-after-free during GTP device teardown

[Cen Zhang (Microsoft)]

gtp_encap_disable() releases the kernel-created sockets while
sk_created is still true and without waiting for in-flight readers.
This allows two concurrent use-after-free scenarios:

1. A softirq packet handler that already observed sk_created == true
   via smp_load_acquire() can dereference sk0/sk1u after they have
   been freed.

2. gtp_genl_send_echo_req() runs in process context without RTNL.
   synchronize_net() waits for RCU-protected softirq handlers but
   does not cover this non-RCU generic netlink reader, which can
   dereference freed sk0/sk1u during concurrent teardown:

     RIP: 0010:ip4_route_output_gtp  (drivers/net/gtp.c)
     gtp_genl_send_echo_req
     Kernel panic - not syncing: Fatal exception

Reorder gtp_encap_disable() to clear sk_created first, then call
synchronize_net() to wait for in-flight softirq handlers before
releasing the sockets. Hold RTNL in gtp_genl_send_echo_req() to
serialize with teardown for the process-context path. Under RTNL,
the smp_load_acquire() from patch 1/2 becomes redundant and is
replaced with a plain read.

Fixes: b20dc3c68458 ("gtp: Allow to create GTP device without FDs")
Fixes: d33bd757d362 ("gtp: Implement GTP echo request")
Reported-by: AutonomousCodeSecurity@microsoft.com
Reported-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
---
v2: New patch. Fix teardown race with synchronize_net() for softirq
    paths and rtnl_lock() for the process-context genl echo path.
v1: https://lore.kernel.org/netdev/20260816035205.57966-1-blbllhy@gmail.com/
---
 drivers/net/gtp.c | 61 ++++++++++++++++++++++++++++++-----------------
 1 file changed, 39 insertions(+), 22 deletions(-)

diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index ead519ee18d1..7ac9764696f2 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -896,12 +896,14 @@ static void gtp_encap_disable_sock(struct sock *sk)
 static void gtp_encap_disable(struct gtp_dev *gtp)
 {
 	if (gtp->sk_created) {
-		udp_tunnel_sock_release(gtp->sk0);
-		udp_tunnel_sock_release(gtp->sk1u);
-		/* Pairs with smp_load_acquire() in the RX and
-		 * genl echo paths.
+		/* Prevent new readers from entering echo handlers,
+		 * then wait for in-flight softirq readers to complete
+		 * before releasing the sockets.
 		 */
 		smp_store_release(&gtp->sk_created, false);
+		synchronize_net();
+		udp_tunnel_sock_release(gtp->sk0);
+		udp_tunnel_sock_release(gtp->sk1u);
 		gtp->sk0 = NULL;
 		gtp->sk1u = NULL;
 	} else {
@@ -1473,8 +1475,7 @@ static int gtp_create_sockets(struct gtp_dev *gtp, const struct nlattr *nla,
 	gtp->sk1u = sk1u;
 
 	/* Ensure sk0/sk1u are visible before sk_created is set.
-	 * Pairs with smp_load_acquire() in the RX and genl
-	 * echo paths.
+	 * Pairs with smp_load_acquire() in the RX echo paths.
 	 */
 	smp_store_release(&gtp->sk_created, true);
 
@@ -2362,6 +2363,7 @@ static int gtp_genl_send_echo_req(struct sk_buff *skb, struct genl_info *info)
 	struct sock *sk;
 	__be16 port;
 	int len;
+	int ret;
 
 	if (!info->attrs[GTPA_VERSION] ||
 	    !info->attrs[GTPA_LINK] ||
@@ -2373,17 +2375,22 @@ static int gtp_genl_send_echo_req(struct sk_buff *skb, struct genl_info *info)
 	dst_ip = nla_get_be32(info->attrs[GTPA_PEER_ADDRESS]);
 	src_ip = nla_get_be32(info->attrs[GTPA_MS_ADDRESS]);
 
+	rtnl_lock();
+
 	gtp = gtp_find_dev(sock_net(skb->sk), info->attrs);
-	if (!gtp)
-		return -ENODEV;
+	if (!gtp) {
+		ret = -ENODEV;
+		goto out_unlock;
+	}
 
-	/* Pairs with smp_store_release() in gtp_create_sockets()
-	 * and gtp_encap_disable().
-	 */
-	if (!smp_load_acquire(&gtp->sk_created))
-		return -EOPNOTSUPP;
-	if (!(gtp->dev->flags & IFF_UP))
-		return -ENETDOWN;
+	if (!gtp->sk_created) {
+		ret = -EOPNOTSUPP;
+		goto out_unlock;
+	}
+	if (!(gtp->dev->flags & IFF_UP)) {
+		ret = -ENETDOWN;
+		goto out_unlock;
+	}
 
 	if (version == GTP_V0) {
 		struct gtp0_header *gtp0_h;
@@ -2392,8 +2399,10 @@ static int gtp_genl_send_echo_req(struct sk_buff *skb, struct genl_info *info)
 			sizeof(struct iphdr) + sizeof(struct udphdr);
 
 		skb_to_send = netdev_alloc_skb_ip_align(gtp->dev, len);
-		if (!skb_to_send)
-			return -ENOMEM;
+		if (!skb_to_send) {
+			ret = -ENOMEM;
+			goto out_unlock;
+		}
 
 		sk = gtp->sk0;
 		port = htons(GTP0_PORT);
@@ -2409,8 +2418,10 @@ static int gtp_genl_send_echo_req(struct sk_buff *skb, struct genl_info *info)
 			sizeof(struct iphdr) + sizeof(struct udphdr);
 
 		skb_to_send = netdev_alloc_skb_ip_align(gtp->dev, len);
-		if (!skb_to_send)
-			return -ENOMEM;
+		if (!skb_to_send) {
+			ret = -ENOMEM;
+			goto out_unlock;
+		}
 
 		sk = gtp->sk1u;
 		port = htons(GTP1U_PORT);
@@ -2420,7 +2431,8 @@ static int gtp_genl_send_echo_req(struct sk_buff *skb, struct genl_info *info)
 		memset(gtp1u_h, 0, sizeof(struct gtp1_header_long));
 		gtp1u_build_echo_msg(gtp1u_h, GTP_ECHO_REQ);
 	} else {
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_unlock;
 	}
 
 	rt = ip4_route_output_gtp(&fl4, sk, dst_ip, src_ip);
@@ -2428,7 +2440,8 @@ static int gtp_genl_send_echo_req(struct sk_buff *skb, struct genl_info *info)
 		netdev_dbg(gtp->dev, "no route for echo request to %pI4\n",
 			   &dst_ip);
 		kfree_skb(skb_to_send);
-		return -ENODEV;
+		ret = -ENODEV;
+		goto out_unlock;
 	}
 
 	local_bh_disable();
@@ -2442,7 +2455,11 @@ static int gtp_genl_send_echo_req(struct sk_buff *skb, struct genl_info *info)
 				    dev_net(gtp->dev)),
 			    false, 0);
 	local_bh_enable();
-	return 0;
+	ret = 0;
+
+out_unlock:
+	rtnl_unlock();
+	return ret;
 }
 
 static const struct nla_policy gtp_genl_policy[GTPA_MAX + 1] = {
-- 
2.55.0

Re: [PATCH net v2 0/2] gtp: fix sk_created race conditions

[Pablo Neira Ayuso]

On Tue, Aug 25, 2026 at 01:24:02AM -0400, Cen Zhang (Microsoft) wrote:
> Two races exist around the sk_created flag in the GTP driver: an
> initialization ordering issue where sk_created is published before the
> socket pointers are assigned, and a teardown lifetime issue where
> concurrent readers can still use the sockets after they are freed. They
> are split into separate patches because they require fundamentally
> different fixes -- memory barriers for ordering vs synchronize_net() and
> RTNL for lifetime.

May I suggest to partial revert of these patches?

 d33bd757d362 ("gtp: Implement GTP echo request")
 9af41cc33471 ("gtp: Implement GTP echo response")

I don't see a use-case for them

I can see a use-case for:

b20dc3c68458 ("gtp: Allow to create GTP device without FDs")

for testing purpose, ie. allowing to create a GTP tunnel without a
userspace program which deals with control plane.

As far as I understood, the reported issues are all related to them.

> Changes since v1 [1]:
> - Use smp_store_release()/smp_load_acquire() for proper memory ordering
>   as suggested by Simon Horman.
> - Add patch 2/2 to fix teardown races in gtp_encap_disable(): reorder
>   to clear sk_created first, add synchronize_net() for softirq paths,
>   and hold RTNL in gtp_genl_send_echo_req() for the process-context
>   path that synchronize_net() cannot protect.
> 
> Patch 1/2 fixes the initialization ordering race.
> Patch 2/2 fixes the teardown lifetime races (softirq + genl).
> 
> [1] https://lore.kernel.org/netdev/20260816035205.57966-1-blbllhy@gmail.com/
> 
> Cen Zhang (Microsoft) (2):
>   gtp: fix sk_created publication race in gtp_create_sockets()
>   gtp: fix use-after-free during GTP device teardown
> 
>  drivers/net/gtp.c | 70 ++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 51 insertions(+), 19 deletions(-)
> 
> 
> base-commit: b70b7b71561f2be1183b6046964d950c2b203bfc
> -- 
> 2.55.0