[PATCH] xfrm: avoid stale tunnel pointers in deferred input callbacks

[PATCH] xfrm: avoid stale tunnel pointers in deferred input callbacks

[Jun Yao]

IP-TFS input can be deferred while its reordering queue waits for a missing
sequence number. The VTI receive callback stores a pointer to the tunnel in
skb->cb, and the device can be removed and freed before the callback runs.

Keep only scalar tunnel metadata in an skb extension. The VTI callbacks
look up the device by ifindex under RCU and check its link configuration
before using its private data. If the VTI no longer exists, drop the
deferred packet. Use the same metadata to preserve tunnel mark handling
and IPv6 tunnel detection, and clear it when a new XFRM receive path starts
and after the callback completes.

Signed-off-by: Jun Yao <yaojun8558363@gmail.com>

---
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 6787fcf6a..f97412b95 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -5044,6 +5044,7 @@ enum skb_ext_id {
 #endif
 #ifdef CONFIG_XFRM
 	SKB_EXT_SEC_PATH,
+	SKB_EXT_XFRM_TUNNEL,
 #endif
 #if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
 	TC_SKB_EXT,
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index 445223a8d..defdd4088 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -382,6 +382,7 @@ struct xfrm_dst_lookup_params {
 };
 
 struct net_device;
+struct ip6_tnl;
 struct xfrm_type;
 struct xfrm_dst;
 struct xfrm_policy_afinfo {
@@ -740,14 +741,52 @@ struct xfrm_tunnel_skb_cb {
 		struct inet_skb_parm h4;
 		struct inet6_skb_parm h6;
 	} header;
+	/* Keep the layout of the other XFRM skb control blocks stable. */
+	unsigned long reserved;
+};
+
+enum xfrm_tunnel_info_type {
+	XFRM_TUNNEL_INFO_VTI4,
+	XFRM_TUNNEL_INFO_VTI6,
+	XFRM_TUNNEL_INFO_IP6,
+};
 
+/* Stored in an skb extension so it survives asynchronous XFRM input. */
+struct xfrm_tunnel_info {
+	int ifindex;
+	__be32 i_key;
+	u8 type;
 	union {
-		struct ip_tunnel *ip4;
-		struct ip6_tnl *ip6;
-	} tunnel;
+		struct {
+			int link;
+			__be32 saddr;
+			__be32 daddr;
+		} ip4;
+		struct {
+			int link;
+			u8 proto;
+			struct in6_addr laddr;
+			struct in6_addr raddr;
+		} ip6;
+	};
 };
 
-#define XFRM_TUNNEL_SKB_CB(__skb) ((struct xfrm_tunnel_skb_cb *)&((__skb)->cb[0]))
+static inline struct xfrm_tunnel_info *
+xfrm_tunnel_info_add(struct sk_buff *skb)
+{
+	return skb_ext_add(skb, SKB_EXT_XFRM_TUNNEL);
+}
+
+static inline const struct xfrm_tunnel_info *
+xfrm_tunnel_info(const struct sk_buff *skb)
+{
+	return skb_ext_find(skb, SKB_EXT_XFRM_TUNNEL);
+}
+
+static inline void xfrm_tunnel_info_clear(struct sk_buff *skb)
+{
+	skb_ext_del(skb, SKB_EXT_XFRM_TUNNEL);
+}
 
 /*
  * This structure is used for the duration where packets are being
@@ -1851,7 +1890,7 @@ int xfrm4_rcv(struct sk_buff *skb);
 
 static inline int xfrm4_rcv_spi(struct sk_buff *skb, int nexthdr, __be32 spi)
 {
-	XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4 = NULL;
+	xfrm_tunnel_info_clear(skb);
 	XFRM_SPI_SKB_CB(skb)->family = AF_INET;
 	XFRM_SPI_SKB_CB(skb)->daddroff = offsetof(struct iphdr, daddr);
 	return xfrm_input(skb, nexthdr, spi, 0);
@@ -2300,15 +2339,17 @@ static inline int xfrm_if_id_put(struct sk_buff *skb, __u32 if_id)
 static inline int xfrm_tunnel_check(struct sk_buff *skb, struct xfrm_state *x,
 				    unsigned int family)
 {
+	const struct xfrm_tunnel_info *info = xfrm_tunnel_info(skb);
 	bool tunnel = false;
 
 	switch(family) {
 	case AF_INET:
-		if (XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4)
+		if (info && info->type == XFRM_TUNNEL_INFO_VTI4)
 			tunnel = true;
 		break;
 	case AF_INET6:
-		if (XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6)
+		if (info && (info->type == XFRM_TUNNEL_INFO_VTI6 ||
+			     info->type == XFRM_TUNNEL_INFO_IP6))
 			tunnel = true;
 		break;
 	}
diff --git a/net/core/skbuff.c b/net/core/skbuff.c
index 8da53d5b7..32f41370d 100644
--- a/net/core/skbuff.c
+++ b/net/core/skbuff.c
@@ -5241,6 +5241,7 @@ static const u8 skb_ext_type_len[] = {
 #endif
 #ifdef CONFIG_XFRM
 	[SKB_EXT_SEC_PATH] = SKB_EXT_CHUNKSIZEOF(struct sec_path),
+	[SKB_EXT_XFRM_TUNNEL] = SKB_EXT_CHUNKSIZEOF(struct xfrm_tunnel_info),
 #endif
 #if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
 	[TC_SKB_EXT] = SKB_EXT_CHUNKSIZEOF(struct tc_skb_ext),
diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c
index abd77162f..fa0a2a73b 100644
--- a/net/ipv4/esp4_offload.c
+++ b/net/ipv4/esp4_offload.c
@@ -81,7 +81,7 @@ static struct sk_buff *esp4_gro_receive(struct list_head *head,
 	if (NAPI_GRO_CB(skb)->proto == IPPROTO_UDP)
 		encap_type = UDP_ENCAP_ESPINUDP;
 
-	XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4 = NULL;
+	xfrm_tunnel_info_clear(skb);
 	XFRM_SPI_SKB_CB(skb)->family = AF_INET;
 	XFRM_SPI_SKB_CB(skb)->daddroff = offsetof(struct iphdr, daddr);
 	XFRM_SPI_SKB_CB(skb)->seq = seq;
diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index c4f14d42d..3594254df 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -58,10 +58,25 @@ static int vti_input(struct sk_buff *skb, int nexthdr, __be32 spi,
 	tunnel = ip_tunnel_lookup(itn, skb->dev->ifindex, flags,
 				  iph->saddr, iph->daddr, 0);
 	if (tunnel) {
+		struct xfrm_tunnel_info *info;
+
 		if (!xfrm4_policy_check(NULL, XFRM_POLICY_IN, skb))
 			goto drop;
 
-		XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4 = tunnel;
+		info = xfrm_tunnel_info_add(skb);
+		if (!info)
+			goto drop;
+
+		*info = (struct xfrm_tunnel_info) {
+			.ifindex = tunnel->dev->ifindex,
+			.i_key = tunnel->parms.i_key,
+			.type = XFRM_TUNNEL_INFO_VTI4,
+			.ip4 = {
+				.link = tunnel->parms.link,
+				.saddr = tunnel->parms.iph.saddr,
+				.daddr = tunnel->parms.iph.daddr,
+			},
+		};
 
 		if (update_skb_dev)
 			skb->dev = tunnel->dev;
@@ -96,18 +111,28 @@ static int vti_rcv_proto(struct sk_buff *skb)
 
 static int vti_rcv_cb(struct sk_buff *skb, int err)
 {
+	const struct xfrm_tunnel_info *info = xfrm_tunnel_info(skb);
 	unsigned short family;
 	struct net_device *dev;
 	struct xfrm_state *x;
 	const struct xfrm_mode *inner_mode;
-	struct ip_tunnel *tunnel = XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4;
+	struct ip_tunnel *tunnel;
 	u32 orig_mark = skb->mark;
 	int ret;
 
-	if (!tunnel)
+	if (!info || info->type != XFRM_TUNNEL_INFO_VTI4)
 		return 1;
 
-	dev = tunnel->dev;
+	dev = dev_get_by_index_rcu(dev_net(skb->dev), info->ifindex);
+	if (!dev || dev->rtnl_link_ops != &vti_link_ops)
+		return err ? 1 : -ENODEV;
+
+	tunnel = netdev_priv(dev);
+	if (tunnel->parms.i_key != info->i_key ||
+	    tunnel->parms.link != info->ip4.link ||
+	    tunnel->parms.iph.saddr != info->ip4.saddr ||
+	    tunnel->parms.iph.daddr != info->ip4.daddr)
+		return err ? 1 : -ENODEV;
 
 	if (err) {
 		DEV_STATS_INC(dev, rx_errors);
@@ -131,7 +156,7 @@ static int vti_rcv_cb(struct sk_buff *skb, int err)
 
 	family = inner_mode->family;
 
-	skb->mark = be32_to_cpu(tunnel->parms.i_key);
+	skb->mark = be32_to_cpu(info->i_key);
 	ret = xfrm_policy_check(NULL, XFRM_POLICY_IN, skb, family);
 	skb->mark = orig_mark;
 
diff --git a/net/ipv4/xfrm4_protocol.c b/net/ipv4/xfrm4_protocol.c
index 4ee624d8e..c31426ea8 100644
--- a/net/ipv4/xfrm4_protocol.c
+++ b/net/ipv4/xfrm4_protocol.c
@@ -65,7 +65,7 @@ int xfrm4_rcv_encap(struct sk_buff *skb, int nexthdr, __be32 spi,
 	struct xfrm4_protocol *handler;
 	struct xfrm4_protocol __rcu **head = proto_handlers(nexthdr);
 
-	XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4 = NULL;
+	xfrm_tunnel_info_clear(skb);
 	XFRM_SPI_SKB_CB(skb)->family = AF_INET;
 	XFRM_SPI_SKB_CB(skb)->daddroff = offsetof(struct iphdr, daddr);
 
@@ -98,7 +98,7 @@ static int xfrm4_esp_rcv(struct sk_buff *skb)
 	int ret;
 	struct xfrm4_protocol *handler;
 
-	XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4 = NULL;
+	xfrm_tunnel_info_clear(skb);
 
 	for_each_protocol_rcu(esp4_handlers, handler)
 		if ((ret = handler->handler(skb)) != -EINVAL)
@@ -126,7 +126,7 @@ static int xfrm4_ah_rcv(struct sk_buff *skb)
 	int ret;
 	struct xfrm4_protocol *handler;
 
-	XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4 = NULL;
+	xfrm_tunnel_info_clear(skb);
 
 	for_each_protocol_rcu(ah4_handlers, handler)
 		if ((ret = handler->handler(skb)) != -EINVAL)
@@ -154,7 +154,7 @@ static int xfrm4_ipcomp_rcv(struct sk_buff *skb)
 	int ret;
 	struct xfrm4_protocol *handler;
 
-	XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4 = NULL;
+	xfrm_tunnel_info_clear(skb);
 
 	for_each_protocol_rcu(ipcomp4_handlers, handler)
 		if ((ret = handler->handler(skb)) != -EINVAL)
diff --git a/net/ipv6/esp6_offload.c b/net/ipv6/esp6_offload.c
index 22895521a..00cdae2c7 100644
--- a/net/ipv6/esp6_offload.c
+++ b/net/ipv6/esp6_offload.c
@@ -110,7 +110,7 @@ static struct sk_buff *esp6_gro_receive(struct list_head *head,
 		goto out;
 
 	IP6CB(skb)->nhoff = nhoff;
-	XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6 = NULL;
+	xfrm_tunnel_info_clear(skb);
 	XFRM_SPI_SKB_CB(skb)->family = AF_INET6;
 	XFRM_SPI_SKB_CB(skb)->daddroff = offsetof(struct ipv6hdr, daddr);
 	XFRM_SPI_SKB_CB(skb)->seq = seq;
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index c79027839..6cf40b000 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -302,6 +302,8 @@ static int vti6_input_proto(struct sk_buff *skb, int nexthdr, __be32 spi,
 	rcu_read_lock();
 	t = vti6_tnl_lookup(dev_net(skb->dev), &ipv6h->saddr, &ipv6h->daddr);
 	if (t) {
+		struct xfrm_tunnel_info *info;
+
 		if (t->parms.proto != IPPROTO_IPV6 && t->parms.proto != 0) {
 			rcu_read_unlock();
 			goto discard;
@@ -319,9 +321,26 @@ static int vti6_input_proto(struct sk_buff *skb, int nexthdr, __be32 spi,
 			goto discard;
 		}
 
+		info = xfrm_tunnel_info_add(skb);
+		if (!info) {
+			rcu_read_unlock();
+			goto discard;
+		}
+
+		*info = (struct xfrm_tunnel_info) {
+			.ifindex = t->dev->ifindex,
+			.i_key = t->parms.i_key,
+			.type = XFRM_TUNNEL_INFO_VTI6,
+			.ip6 = {
+				.link = t->parms.link,
+				.proto = t->parms.proto,
+				.laddr = t->parms.laddr,
+				.raddr = t->parms.raddr,
+			},
+		};
+
 		rcu_read_unlock();
 
-		XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6 = t;
 		XFRM_SPI_SKB_CB(skb)->family = AF_INET6;
 		XFRM_SPI_SKB_CB(skb)->daddroff = offsetof(struct ipv6hdr, daddr);
 		return xfrm_input(skb, nexthdr, spi, encap_type);
@@ -342,18 +361,29 @@ static int vti6_rcv(struct sk_buff *skb)
 
 static int vti6_rcv_cb(struct sk_buff *skb, int err)
 {
+	const struct xfrm_tunnel_info *info = xfrm_tunnel_info(skb);
 	unsigned short family;
 	struct net_device *dev;
 	struct xfrm_state *x;
 	const struct xfrm_mode *inner_mode;
-	struct ip6_tnl *t = XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6;
+	struct ip6_tnl *t;
 	u32 orig_mark = skb->mark;
 	int ret;
 
-	if (!t)
+	if (!info || info->type != XFRM_TUNNEL_INFO_VTI6)
 		return 1;
 
-	dev = t->dev;
+	dev = dev_get_by_index_rcu(dev_net(skb->dev), info->ifindex);
+	if (!dev || dev->rtnl_link_ops != &vti6_link_ops)
+		return err ? 1 : -ENODEV;
+
+	t = netdev_priv(dev);
+	if (t->parms.i_key != info->i_key ||
+	    t->parms.link != info->ip6.link ||
+	    t->parms.proto != info->ip6.proto ||
+	    !ipv6_addr_equal(&t->parms.laddr, &info->ip6.laddr) ||
+	    !ipv6_addr_equal(&t->parms.raddr, &info->ip6.raddr))
+		return err ? 1 : -ENODEV;
 
 	if (err) {
 		DEV_STATS_INC(dev, rx_errors);
@@ -377,7 +407,7 @@ static int vti6_rcv_cb(struct sk_buff *skb, int err)
 
 	family = inner_mode->family;
 
-	skb->mark = be32_to_cpu(t->parms.i_key);
+	skb->mark = be32_to_cpu(info->i_key);
 	ret = xfrm_policy_check(NULL, XFRM_POLICY_IN, skb, family);
 	skb->mark = orig_mark;
 
diff --git a/net/ipv6/xfrm6_input.c b/net/ipv6/xfrm6_input.c
index 07edef258..9aed0b21d 100644
--- a/net/ipv6/xfrm6_input.c
+++ b/net/ipv6/xfrm6_input.c
@@ -15,6 +15,7 @@
 #include <linux/netfilter.h>
 #include <linux/netfilter_ipv6.h>
 #include <net/ipv6.h>
+#include <net/ip6_tunnel.h>
 #include <net/xfrm.h>
 #include <net/protocol.h>
 #include <net/gro.h>
@@ -22,7 +23,28 @@
 int xfrm6_rcv_spi(struct sk_buff *skb, int nexthdr, __be32 spi,
 		  struct ip6_tnl *t)
 {
-	XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6 = t;
+	struct xfrm_tunnel_info *info;
+
+	xfrm_tunnel_info_clear(skb);
+	if (t) {
+		info = xfrm_tunnel_info_add(skb);
+		if (!info) {
+			kfree_skb(skb);
+			return 0;
+		}
+
+		*info = (struct xfrm_tunnel_info) {
+			.ifindex = t->dev->ifindex,
+			.i_key = t->parms.i_key,
+			.type = XFRM_TUNNEL_INFO_IP6,
+			.ip6 = {
+				.link = t->parms.link,
+				.proto = t->parms.proto,
+				.laddr = t->parms.laddr,
+				.raddr = t->parms.raddr,
+			},
+		};
+	}
 	XFRM_SPI_SKB_CB(skb)->family = AF_INET6;
 	XFRM_SPI_SKB_CB(skb)->daddroff = offsetof(struct ipv6hdr, daddr);
 	return xfrm_input(skb, nexthdr, spi, 0);
diff --git a/net/ipv6/xfrm6_protocol.c b/net/ipv6/xfrm6_protocol.c
index 9b586fcec..2e93029a3 100644
--- a/net/ipv6/xfrm6_protocol.c
+++ b/net/ipv6/xfrm6_protocol.c
@@ -66,7 +66,7 @@ int xfrm6_rcv_encap(struct sk_buff *skb, int nexthdr, __be32 spi,
 	struct xfrm6_protocol *handler;
 	struct xfrm6_protocol __rcu **head = proto_handlers(nexthdr);
 
-	XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6 = NULL;
+	xfrm_tunnel_info_clear(skb);
 	XFRM_SPI_SKB_CB(skb)->family = AF_INET6;
 	XFRM_SPI_SKB_CB(skb)->daddroff = offsetof(struct ipv6hdr, daddr);
 
@@ -113,7 +113,7 @@ static int xfrm6_esp_rcv(struct sk_buff *skb)
 	int ret;
 	struct xfrm6_protocol *handler;
 
-	XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6 = NULL;
+	xfrm_tunnel_info_clear(skb);
 
 	for_each_protocol_rcu(esp6_handlers, handler)
 		if ((ret = handler->handler(skb)) != -EINVAL)
@@ -142,7 +142,7 @@ static int xfrm6_ah_rcv(struct sk_buff *skb)
 	int ret;
 	struct xfrm6_protocol *handler;
 
-	XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6 = NULL;
+	xfrm_tunnel_info_clear(skb);
 
 	for_each_protocol_rcu(ah6_handlers, handler)
 		if ((ret = handler->handler(skb)) != -EINVAL)
@@ -171,7 +171,7 @@ static int xfrm6_ipcomp_rcv(struct sk_buff *skb)
 	int ret;
 	struct xfrm6_protocol *handler;
 
-	XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6 = NULL;
+	xfrm_tunnel_info_clear(skb);
 
 	for_each_protocol_rcu(ipcomp6_handlers, handler)
 		if ((ret = handler->handler(skb)) != -EINVAL)
diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
index 5ed87d513..a5d67d506 100644
--- a/net/xfrm/xfrm_input.c
+++ b/net/xfrm/xfrm_input.c
@@ -466,6 +466,7 @@ static int xfrm_inner_mode_input(struct xfrm_state *x,
 int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
 {
 	const struct xfrm_state_afinfo *afinfo;
+	const struct xfrm_tunnel_info *tunnel_info;
 	struct net *net = dev_net(skb->dev);
 	struct net_device *dev = skb->dev;
 	int err;
@@ -555,16 +556,20 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
 	}
 
 	family = XFRM_SPI_SKB_CB(skb)->family;
+	tunnel_info = xfrm_tunnel_info(skb);
 
 	/* if tunnel is present override skb->mark value with tunnel i_key */
 	switch (family) {
 	case AF_INET:
-		if (XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4)
-			mark = be32_to_cpu(XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4->parms.i_key);
+		if (tunnel_info &&
+		    tunnel_info->type == XFRM_TUNNEL_INFO_VTI4)
+			mark = be32_to_cpu(tunnel_info->i_key);
 		break;
 	case AF_INET6:
-		if (XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6)
-			mark = be32_to_cpu(XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6->parms.i_key);
+		if (tunnel_info &&
+		    (tunnel_info->type == XFRM_TUNNEL_INFO_VTI6 ||
+		     tunnel_info->type == XFRM_TUNNEL_INFO_IP6))
+			mark = be32_to_cpu(tunnel_info->i_key);
 		break;
 	}
 
@@ -739,6 +744,7 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
 		goto drop;
 	}
 
+	xfrm_tunnel_info_clear(skb);
 	nf_reset_ct(skb);
 
 	if (decaps) {
diff --git a/net/xfrm/xfrm_interface_core.c b/net/xfrm/xfrm_interface_core.c
index 9835fbe63..d91b5f22d 100644
--- a/net/xfrm/xfrm_interface_core.c
+++ b/net/xfrm/xfrm_interface_core.c
@@ -322,10 +322,10 @@ static int xfrmi_input(struct sk_buff *skb, int nexthdr, __be32 spi,
 	XFRM_SPI_SKB_CB(skb)->family = family;
 	if (family == AF_INET) {
 		XFRM_SPI_SKB_CB(skb)->daddroff = offsetof(struct iphdr, daddr);
-		XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4 = NULL;
+		xfrm_tunnel_info_clear(skb);
 	} else {
 		XFRM_SPI_SKB_CB(skb)->daddroff = offsetof(struct ipv6hdr, daddr);
-		XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6 = NULL;
+		xfrm_tunnel_info_clear(skb);
 	}
 
 	return xfrm_input(skb, nexthdr, spi, encap_type);

Re: [PATCH] xfrm: avoid stale tunnel pointers in deferred input callbacks

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by LLM and not proven at runtime). If it was triggered
   please include the symptoms, like the stack trace or kernel messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH] xfrm: avoid stale tunnel pointers in deferred input callbacks

[Victor Nogueira]

Hi!

On 11/11/2026 06:46, Jun Yao wrote:
> IP-TFS input can be deferred while its reordering queue waits for a missing
> sequence number. The VTI receive callback stores a pointer to the tunnel in
> skb->cb, and the device can be removed and freed before the callback runs.
> 
> Keep only scalar tunnel metadata in an skb extension. The VTI callbacks
> look up the device by ifindex under RCU and check its link configuration
> before using its private data. If the VTI no longer exists, drop the
> deferred packet. Use the same metadata to preserve tunnel mark handling
> and IPv6 tunnel detection, and clear it when a new XFRM receive path starts
> and after the callback completes.
> 
> Signed-off-by: Jun Yao <yaojun8558363@gmail.com>
> 
> ---
> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 6787fcf6a..f97412b95 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -5044,6 +5044,7 @@ enum skb_ext_id {
>   #endif
>   #ifdef CONFIG_XFRM
>   	SKB_EXT_SEC_PATH,
> +	SKB_EXT_XFRM_TUNNEL,
>   #endif
>   #if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
>   	TC_SKB_EXT,
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index 445223a8d..defdd4088 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> [...]
>   
> -#define XFRM_TUNNEL_SKB_CB(__skb) ((struct xfrm_tunnel_skb_cb *)&((__skb)->cb[0]))
> +static inline struct xfrm_tunnel_info *
> +xfrm_tunnel_info_add(struct sk_buff *skb)
> +{
> +	return skb_ext_add(skb, SKB_EXT_XFRM_TUNNEL);
> +}
> +
> +static inline const struct xfrm_tunnel_info *
> +xfrm_tunnel_info(const struct sk_buff *skb)
> +{
> +	return skb_ext_find(skb, SKB_EXT_XFRM_TUNNEL);
> +}
> +
> +static inline void xfrm_tunnel_info_clear(struct sk_buff *skb)
> +{
> +	skb_ext_del(skb, SKB_EXT_XFRM_TUNNEL);
> +}

The above lines are causing build failures when XFRM is not enabled:

In file included from net/core/sock.c:134:
./include/net/xfrm.h: In function ‘xfrm_tunnel_info_add’:
./include/net/xfrm.h:777:33: error: ‘SKB_EXT_XFRM_TUNNEL’ undeclared 
(first use in this function); did you mean ‘DST_XFRM_TUNNEL’?
   777 |         return skb_ext_add(skb, SKB_EXT_XFRM_TUNNEL);
       |                                 ^~~~~~~~~~~~~~~~~~~
       |                                 DST_XFRM_TUNNEL
./include/net/xfrm.h:777:33: note: each undeclared identifier is 
reported only once for each function it appears in
./include/net/xfrm.h: In function ‘xfrm_tunnel_info’:
./include/net/xfrm.h:783:34: error: ‘SKB_EXT_XFRM_TUNNEL’ undeclared 
(first use in this function); did you mean ‘DST_XFRM_TUNNEL’?
   783 |         return skb_ext_find(skb, SKB_EXT_XFRM_TUNNEL);
       |                                  ^~~~~~~~~~~~~~~~~~~
       |                                  DST_XFRM_TUNNEL
./include/net/xfrm.h: In function ‘xfrm_tunnel_info_clear’:
./include/net/xfrm.h:788:26: error: ‘SKB_EXT_XFRM_TUNNEL’ undeclared 
(first use in this function); did you mean ‘DST_XFRM_TUNNEL’?
   788 |         skb_ext_del(skb, SKB_EXT_XFRM_TUNNEL);
       |                          ^~~~~~~~~~~~~~~~~~~
       |                          DST_XFRM_TUNNEL

---
pw-bot: cr

Re: [PATCH] xfrm: avoid stale tunnel pointers in deferred input callbacks

[kernel test robot]

Hi Jun,

kernel test robot noticed the following build errors:

[auto build test ERROR on klassert-ipsec-next/master]
[also build test ERROR on klassert-ipsec/master net/main net-next/main linus/master v7.4-rc2 next-20261110]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jun-Yao/xfrm-avoid-stale-tunnel-pointers-in-deferred-input-callbacks/20261111-174629
base:   https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master
patch link:    https://lore.kernel.org/r/20261111094629.230568-1-yaojun8558363%40gmail.com
patch subject: [PATCH] xfrm: avoid stale tunnel pointers in deferred input callbacks
config: nios2-defconfig (https://download.01.org/0day-ci/archive/20261112/202611120519.EG3LbKfu-lkp@intel.com/config)
compiler: nios2-linux-gcc (GCC) 11.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261112/202611120519.EG3LbKfu-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202611120519.EG3LbKfu-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from net/core/sock.c:134:
   include/net/xfrm.h: In function 'xfrm_tunnel_info_add':
>> include/net/xfrm.h:777:16: error: implicit declaration of function 'skb_ext_add'; did you mean 'skb_ext_del'? [-Werror=implicit-function-declaration]
     777 |         return skb_ext_add(skb, SKB_EXT_XFRM_TUNNEL);
         |                ^~~~~~~~~~~
         |                skb_ext_del
>> include/net/xfrm.h:777:33: error: 'SKB_EXT_XFRM_TUNNEL' undeclared (first use in this function); did you mean 'DST_XFRM_TUNNEL'?
     777 |         return skb_ext_add(skb, SKB_EXT_XFRM_TUNNEL);
         |                                 ^~~~~~~~~~~~~~~~~~~
         |                                 DST_XFRM_TUNNEL
   include/net/xfrm.h:777:33: note: each undeclared identifier is reported only once for each function it appears in
   include/net/xfrm.h: In function 'xfrm_tunnel_info':
>> include/net/xfrm.h:783:16: error: implicit declaration of function 'skb_ext_find'; did you mean 'skb_ext_copy'? [-Werror=implicit-function-declaration]
     783 |         return skb_ext_find(skb, SKB_EXT_XFRM_TUNNEL);
         |                ^~~~~~~~~~~~
         |                skb_ext_copy
   include/net/xfrm.h:783:34: error: 'SKB_EXT_XFRM_TUNNEL' undeclared (first use in this function); did you mean 'DST_XFRM_TUNNEL'?
     783 |         return skb_ext_find(skb, SKB_EXT_XFRM_TUNNEL);
         |                                  ^~~~~~~~~~~~~~~~~~~
         |                                  DST_XFRM_TUNNEL
   include/net/xfrm.h: In function 'xfrm_tunnel_info_clear':
   include/net/xfrm.h:788:26: error: 'SKB_EXT_XFRM_TUNNEL' undeclared (first use in this function); did you mean 'DST_XFRM_TUNNEL'?
     788 |         skb_ext_del(skb, SKB_EXT_XFRM_TUNNEL);
         |                          ^~~~~~~~~~~~~~~~~~~
         |                          DST_XFRM_TUNNEL
   cc1: some warnings being treated as errors


vim +777 include/net/xfrm.h

   773	
   774	static inline struct xfrm_tunnel_info *
   775	xfrm_tunnel_info_add(struct sk_buff *skb)
   776	{
 > 777		return skb_ext_add(skb, SKB_EXT_XFRM_TUNNEL);
   778	}
   779	
   780	static inline const struct xfrm_tunnel_info *
   781	xfrm_tunnel_info(const struct sk_buff *skb)
   782	{
 > 783		return skb_ext_find(skb, SKB_EXT_XFRM_TUNNEL);
   784	}
   785	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] xfrm: avoid stale tunnel pointers in deferred input callbacks

[kernel test robot]

Hi Jun,

kernel test robot noticed the following build errors:

[auto build test ERROR on klassert-ipsec-next/master]
[also build test ERROR on klassert-ipsec/master net/main net-next/main linus/master v7.4-rc2 next-20261110]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Jun-Yao/xfrm-avoid-stale-tunnel-pointers-in-deferred-input-callbacks/20261111-174629
base:   https://git.kernel.org/pub/scm/linux/kernel/git/klassert/ipsec-next.git master
patch link:    https://lore.kernel.org/r/20261111094629.230568-1-yaojun8558363%40gmail.com
patch subject: [PATCH] xfrm: avoid stale tunnel pointers in deferred input callbacks
config: mips-xway_defconfig (https://download.01.org/0day-ci/archive/20261112/202611120624.jcKHEACy-lkp@intel.com/config)
compiler: clang version 19.1.7 (https://github.com/llvm/llvm-project cd708029e0b2869e80abe31ddb175f7c35361f90)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261112/202611120624.jcKHEACy-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202611120624.jcKHEACy-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from net/core/sock.c:134:
   include/net/xfrm.h:777:9: error: call to undeclared function 'skb_ext_add'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     777 |         return skb_ext_add(skb, SKB_EXT_XFRM_TUNNEL);
         |                ^
   include/net/xfrm.h:777:26: error: use of undeclared identifier 'SKB_EXT_XFRM_TUNNEL'; did you mean 'SKB_GSO_UDP_TUNNEL'?
     777 |         return skb_ext_add(skb, SKB_EXT_XFRM_TUNNEL);
         |                                 ^~~~~~~~~~~~~~~~~~~
         |                                 SKB_GSO_UDP_TUNNEL
   include/linux/skbuff.h:697:2: note: 'SKB_GSO_UDP_TUNNEL' declared here
     697 |         SKB_GSO_UDP_TUNNEL = 1 << 10,
         |         ^
   In file included from net/core/sock.c:134:
>> include/net/xfrm.h:777:9: error: incompatible integer to pointer conversion returning 'int' from a function with result type 'struct xfrm_tunnel_info *' [-Wint-conversion]
     777 |         return skb_ext_add(skb, SKB_EXT_XFRM_TUNNEL);
         |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/net/xfrm.h:783:9: error: call to undeclared function 'skb_ext_find'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     783 |         return skb_ext_find(skb, SKB_EXT_XFRM_TUNNEL);
         |                ^
   include/net/xfrm.h:783:27: error: use of undeclared identifier 'SKB_EXT_XFRM_TUNNEL'; did you mean 'SKB_GSO_UDP_TUNNEL'?
     783 |         return skb_ext_find(skb, SKB_EXT_XFRM_TUNNEL);
         |                                  ^~~~~~~~~~~~~~~~~~~
         |                                  SKB_GSO_UDP_TUNNEL
   include/linux/skbuff.h:697:2: note: 'SKB_GSO_UDP_TUNNEL' declared here
     697 |         SKB_GSO_UDP_TUNNEL = 1 << 10,
         |         ^
   In file included from net/core/sock.c:134:
>> include/net/xfrm.h:783:9: error: incompatible integer to pointer conversion returning 'int' from a function with result type 'const struct xfrm_tunnel_info *' [-Wint-conversion]
     783 |         return skb_ext_find(skb, SKB_EXT_XFRM_TUNNEL);
         |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   include/net/xfrm.h:788:19: error: use of undeclared identifier 'SKB_EXT_XFRM_TUNNEL'; did you mean 'SKB_GSO_UDP_TUNNEL'?
     788 |         skb_ext_del(skb, SKB_EXT_XFRM_TUNNEL);
         |                          ^~~~~~~~~~~~~~~~~~~
         |                          SKB_GSO_UDP_TUNNEL
   include/linux/skbuff.h:697:2: note: 'SKB_GSO_UDP_TUNNEL' declared here
     697 |         SKB_GSO_UDP_TUNNEL = 1 << 10,
         |         ^
   7 errors generated.


vim +777 include/net/xfrm.h

   773	
   774	static inline struct xfrm_tunnel_info *
   775	xfrm_tunnel_info_add(struct sk_buff *skb)
   776	{
 > 777		return skb_ext_add(skb, SKB_EXT_XFRM_TUNNEL);
   778	}
   779	
   780	static inline const struct xfrm_tunnel_info *
   781	xfrm_tunnel_info(const struct sk_buff *skb)
   782	{
 > 783		return skb_ext_find(skb, SKB_EXT_XFRM_TUNNEL);
   784	}
   785	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH] xfrm: avoid stale tunnel pointers in deferred input callbacks

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 9 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 1 · Low: 6

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The VTI callbacks look up the tunnel device in the wrong network
  namespace.
- [High] The underlay net_device pointer still dangles across the IP-TFS
  deferral, and the new lookup depends on it.
- [Medium] The new skb extension ID SKB_EXT_XFRM_TUNNEL takes the last
  available slot and makes every skb extension object bigger, and the…
- [Low] The build fails whenever CONFIG_XFRM=n.
- [Low] IP-TFS inner packets in freshly allocated skbs now get VTI
  processing, which is a behaviour change the commit message does not…
- [Low] The commit fixes a user-triggerable use-after-free but has no
  Fixes: tag (and no Cc: stable).
- [Low] The XFRM_TUNNEL_INFO_IP6 branch added to xfrm6_rcv_spi() is dead
  code, and its payload is never read.
- [Low] The new identity checks in vti_rcv_cb() and vti6_rcv_cb() read
  tunnel->parms fields (i_key, link, iph.saddr/daddr; or link, proto,…
- [Low] xfrm: vti_rcv_cb() and vti6_rcv_cb() return 1 on error when dev
  is gone

--

xfrm: avoid stale tunnel pointers in deferred input callbacks

This patch removes the ip_tunnel/ip6_tnl pointer that VTI stored in
skb->cb. In its place, scalar tunnel metadata goes into a new
SKB_EXT_XFRM_TUNNEL skb extension. The VTI receive callbacks then look up
the tunnel device by ifindex under RCU and recheck its parameters. If the
tunnel no longer matches, the packet is dropped.

> IP-TFS input can be deferred while its reordering queue waits for a missing
> sequence number. The VTI receive callback stores a pointer to the tunnel in
> skb->cb, and the device can be removed and freed before the callback runs.

[ ... ]

> Signed-off-by: Jun Yao <yaojun8558363@gmail.com>

[Severity: Low]
This fixes a use-after-free that users can trigger, but there is no Fixes:
tag or Cc: stable. Could one be added so stable maintainers know which
trees need the backport?

The problem being fixed does exist. In __input_process_payload(), the
"Reuse the received skb" path keeps the original skb->cb while the skb
sits in w_saved or waits for the drop_timer. That cb includes the raw
tunnel pointer stored by vti_input()/vti6_input_proto().

Nothing holds a reference on the VTI device during that wait, and
vti_rcv_cb() dereferences the pointer later. Async crypto completion
opens a similar window.

> diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
> index 6787fcf6ac3e4..f97412b95c093 100644
> --- a/include/linux/skbuff.h
> +++ b/include/linux/skbuff.h
> @@ -5044,6 +5044,7 @@ enum skb_ext_id {
>  #endif
>  #ifdef CONFIG_XFRM
>  	SKB_EXT_SEC_PATH,
> +	SKB_EXT_XFRM_TUNNEL,
>  #endif
>  #if IS_ENABLED(CONFIG_NET_TC_SKB_EXT)
>  	TC_SKB_EXT,

[Severity: Medium]
Does this use up the last skb extension ID? With BRIDGE_NETFILTER, XFRM,
NET_TC_SKB_EXT, MPTCP, MCTP_FLOWS, INET_PSP and CAN all enabled, enum
skb_ext_id now has 8 entries. That is the limit checked in
skb_extensions_init():

	BUILD_BUG_ON(SKB_EXT_NUM > 8);

No further extension can be added without widening active_extensions.

There is also a size cost. skbuff_ext_cache objects are sized as
SKB_EXT_ALIGN_VALUE * skb_ext_total_length(), which is the sum over all
types. struct xfrm_tunnel_info is 52 bytes, which rounds up to 56. So the
new skb_ext_type_len[] entry makes every extension allocation in the
system 56 bytes larger, including sec_path, MPTCP, TC and CAN extensions.

The VTI and xfrm6 tunnel input paths also change from a plain cb store to
a GFP_ATOMIC skb_ext_add() or COW, with a new drop path when it fails.

Would a narrower fix avoid this system-wide cost, for example holding a
tracked netdev reference across the deferral? The commit message does not
mention any of these tradeoffs.

> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index 445223a8dc6f1..defdd4088c552 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h

[ ... ]

> @@ -740,14 +741,52 @@ struct xfrm_tunnel_skb_cb {

[ ... ]

> -#define XFRM_TUNNEL_SKB_CB(__skb) ((struct xfrm_tunnel_skb_cb *)&((__skb)->cb[0]))
> +static inline struct xfrm_tunnel_info *
> +xfrm_tunnel_info_add(struct sk_buff *skb)
> +{
> +	return skb_ext_add(skb, SKB_EXT_XFRM_TUNNEL);
> +}
> +
> +static inline const struct xfrm_tunnel_info *
> +xfrm_tunnel_info(const struct sk_buff *skb)
> +{
> +	return skb_ext_find(skb, SKB_EXT_XFRM_TUNNEL);
> +}
> +
> +static inline void xfrm_tunnel_info_clear(struct sk_buff *skb)
> +{
> +	skb_ext_del(skb, SKB_EXT_XFRM_TUNNEL);
> +}

[Severity: Low]
Does this build with CONFIG_XFRM=n? These inlines are not under any config
guard. SKB_EXT_XFRM_TUNNEL only exists under CONFIG_SKB_EXTENSIONS plus
CONFIG_XFRM. Also, skb_ext_add() and skb_ext_find() have no stubs in the
!CONFIG_SKB_EXTENSIONS branch of skbuff.h.

xfrm4_rcv_spi() and xfrm_tunnel_check() are always compiled and call
these helpers. Files like net/ipv4/ip_input.c, route.c, tcp_ipv4.c and
udp.c include <net/xfrm.h> unconditionally. CONFIG_XFRM is a hidden bool
that is only selected, so INET=y with XFRM=n is a valid config.

skb_sec_path() handles this with an #ifdef CONFIG_XFRM / return NULL
fallback. Could the new helpers do the same?

[ ... ]

> diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
> index c4f14d42df248..3594254df9de9 100644
> --- a/net/ipv4/ip_vti.c
> +++ b/net/ipv4/ip_vti.c
> @@ -58,10 +58,25 @@ static int vti_input(struct sk_buff *skb, int nexthdr, __be32 spi,

[ ... ]

> +		*info = (struct xfrm_tunnel_info) {
> +			.ifindex = tunnel->dev->ifindex,
> +			.i_key = tunnel->parms.i_key,
> +			.type = XFRM_TUNNEL_INFO_VTI4,
> +			.ip4 = {
> +				.link = tunnel->parms.link,
> +				.saddr = tunnel->parms.iph.saddr,
> +				.daddr = tunnel->parms.iph.daddr,
> +			},
> +		};
>
>  		if (update_skb_dev)
>  			skb->dev = tunnel->dev;

[ ... ]

> @@ -96,18 +111,28 @@ static int vti_rcv_proto(struct sk_buff *skb)
>
>  static int vti_rcv_cb(struct sk_buff *skb, int err)
>  {
> +	const struct xfrm_tunnel_info *info = xfrm_tunnel_info(skb);
>  	unsigned short family;
>  	struct net_device *dev;
>  	struct xfrm_state *x;
>  	const struct xfrm_mode *inner_mode;
> -	struct ip_tunnel *tunnel = XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip4;
> +	struct ip_tunnel *tunnel;
>  	u32 orig_mark = skb->mark;
>  	int ret;
>
> -	if (!tunnel)
> +	if (!info || info->type != XFRM_TUNNEL_INFO_VTI4)
>  		return 1;

[Severity: Low]
Is it intended that this changes which IP-TFS inner packets get VTI
processing? The commit message does not mention it.

iptfs_alloc_skb() allocates inner skbs with a zeroed cb. From the outer
skb it copies only dev and the extensions:

net/xfrm/xfrm_iptfs.c:iptfs_alloc_skb() {
    ...
		skb->dev = tpl->dev;
		__skb_ext_copy(skb, tpl);
    ...
}

Before this patch the cb tunnel pointer was NULL for those skbs, so
vti_rcv_cb()/vti6_rcv_cb() returned 1. They skipped the VTI policy check,
the switch of skb->dev to the VTI device, and the VTI stats. Only the
reused outer skb kept the pointer.

Now every inner skb gets the tunnel info through the shared extension.
They all go through xfrm_policy_check() with the i_key mark, then
skb_scrub_packet() and skb->dev = dev. As a result they can fail with
-EPERM, or appear on a different netdev than before.

Deferred packets are also now dropped when the VTI's i_key, link or
addresses change, not only when the VTI disappears as the commit message
says.

The new behaviour may be more consistent. Should the commit message
describe it?

>
> -	dev = tunnel->dev;
> +	dev = dev_get_by_index_rcu(dev_net(skb->dev), info->ifindex);
> +	if (!dev || dev->rtnl_link_ops != &vti_link_ops)
> +		return err ? 1 : -ENODEV;

[Severity: Low]
If the VTI device is gone or its parameters no longer match, should
the error call return 1 here?

When info->type is XFRM_TUNNEL_INFO_VTI4, vti_input() has already
claimed this packet. The cb_handler dispatchers in xfrm4_rcv_cb() and
tunnel4_rcv_cb() call the handlers in priority order and stop at the
first one that returns <= 0. A positive return means "not mine, keep
going".

Here is the sequence when the device disappears before the callback
runs:

xfrm_input()
  xfrm_rcv_cb(skb, family, xfrm_proto, 0)
    vti_rcv_cb(skb, 0)     -> lookup fails, returns -ENODEV
  goto drop
  xfrm_rcv_cb(skb, family, xfrm_proto, -1)
    vti_rcv_cb(skb, -1)    -> lookup fails again, returns 1
    xfrmi_rcv_cb(skb, -1)  -> next handler is consulted

The extension is still attached at that point, because
xfrm_tunnel_info_clear() only runs after a successful callback. An
earlier transform failure that jumps straight to the drop label ends
the same way.

The return value of 1 sends the dispatcher on to lower-priority
handlers such as xfrmi_rcv_cb(). If an xfrmi device matches by if_id
or collect_md, its rx_errors and rx_dropped get incremented for a
packet that belonged to the VTI. Before this patch, a non-NULL tunnel
pointer meant the error path always returned 0.

vti6_rcv_cb() has the same pattern with xfrm6_rcv_cb() and
tunnel6_rcv_cb().

Would it be better to return 0 whenever err is set and the extension
says VTI4/VTI6, and simply skip the stats update when the device can
no longer be found?

[Severity: High]
Is this lookup done in the right network namespace?

info->ifindex comes from tunnel->dev->ifindex, so it is an index in
dev_net(tunnel->dev). Here skb->dev is still the underlay device, for two
reasons. Every vti_input() caller passes update_skb_dev as false. On the
IP-TFS path, iptfs_alloc_skb() copies tpl->dev. So dev_net(skb->dev) is
the tunnel's link netns, not the netns the VTI netdevice lives in.

Cross-netns VTI is a supported setup:

net/ipv4/ip_vti.c:vti_newlink() {
    ...
	return ip_tunnel_newlink(params->link_net ? : dev_net(dev), dev, tb,
				 &parms, fwmark);
}

vti6_newlink() also sets nt->net = params->link_net ? : dev_net(dev).
Only the fallback devices are netns_immutable. A vti/vti6 device can be
created with a link netns, or moved with "ip link set vtiX netns B".

In that setup the lookup returns NULL and the callback returns -ENODEV.
xfrm_input() then drops the packet:

	err = xfrm_rcv_cb(skb, family, xfrm_proto, 0);
	if (err) {
		rcu_read_unlock();
		goto drop;
	}

That means every decapsulated packet on such a tunnel is thrown away,
including on the normal synchronous path. Before this patch these packets
were delivered through tunnel->dev. The remaining check still expects the
two namespaces to be able to differ:

	skb_scrub_packet(skb, !net_eq(tunnel->net, dev_net(skb->dev)));

The underlay netns could also hold a different vti device with the same
ifindex, for example one moved in from a third netns. Its tunnel is in a
different itn, so the -EEXIST duplicate check does not cover it. If its
i_key, link and addresses match, could the packet be attributed to that
device and injected into the wrong namespace?

vti6_rcv_cb() does the same lookup with dev_net(skb->dev), so it seems to
have the same problem.

[Severity: High]
On the deferred path, is skb->dev itself still safe to dereference here?

On the IP-TFS path the commit message describes, skb->dev is the underlay
device and nothing holds a reference on it. Packets wait in
xtfs->w_saved[] or ra_newskb until the drop_timer or a later in-order
packet drains them.

xfrm_iptfs.c has no dev_hold() or netdev_hold(), and xfrm_input() drops
its dev_hold() once x->type->input() returns -EINPROGRESS.
xfrm_dev_unregister() only flushes states offloaded to that device.
Software IP-TFS states and their queued skbs therefore survive
unregistration.

Suppose the underlay is deleted while packets are queued, for example
with "ip link del veth0". The drain path then reads freed memory:

iptfs_drop_timer() / iptfs_input_ordered()
  ...
    __input_process_payload()
      xfrm_input(skb, 0, 0, -2)
        net = dev_net(skb->dev)
        ...
        xfrm_rcv_cb()
          vti_rcv_cb()
            dev_get_by_index_rcu(dev_net(skb->dev), ...)

gro_cells_receive() uses skb->dev as well.

The dereference at xfrm_input() entry was there before this patch.
However, the patch is meant to fix "the device can be removed and freed
before the callback runs" on this exact path. The new lookup depends on
the same unreferenced pointer.

For the fix to be complete, does the device or the netns need to be
pinned across the deferral? That could be netdev_hold()/netdev_put(), or
storing a netns identity.

> +
> +	tunnel = netdev_priv(dev);
> +	if (tunnel->parms.i_key != info->i_key ||
> +	    tunnel->parms.link != info->ip4.link ||
> +	    tunnel->parms.iph.saddr != info->ip4.saddr ||
> +	    tunnel->parms.iph.daddr != info->ip4.daddr)
> +		return err ? 1 : -ENODEV;

[Severity: Low]
Can these reads race with tunnel reconfiguration? They are plain loads
under RCU only. Meanwhile ip_tunnel_update() rewrites saddr, daddr and
i_key in place under RTNL, and updates link with:

	WRITE_ONCE(t->parms.link, p->link);

Lockless readers such as ip_tunnel_lookup() use READ_ONCE(t->parms.link).
The new reads of parms.link here and in vti_input() do not, which KCSAN
would report.

vti6_tnl_change() also rewrites the 16-byte laddr/raddr with plain struct
stores, and vti6_rcv_cb() does not wait out the synchronize_net() in
vti6_update(). If the callback sees a half-updated parms set, packets in
flight (deferred or not) could be dropped with -ENODEV during
reconfiguration.

[ ... ]

> diff --git a/net/ipv6/xfrm6_input.c b/net/ipv6/xfrm6_input.c
> index 07edef2589844..9aed0b21d3b02 100644
> --- a/net/ipv6/xfrm6_input.c
> +++ b/net/ipv6/xfrm6_input.c

[ ... ]

> @@ -22,7 +23,28 @@
>  int xfrm6_rcv_spi(struct sk_buff *skb, int nexthdr, __be32 spi,
>  		  struct ip6_tnl *t)
>  {
> -	XFRM_TUNNEL_SKB_CB(skb)->tunnel.ip6 = t;
> +	struct xfrm_tunnel_info *info;
> +
> +	xfrm_tunnel_info_clear(skb);
> +	if (t) {
> +		info = xfrm_tunnel_info_add(skb);
> +		if (!info) {
> +			kfree_skb(skb);
> +			return 0;
> +		}
> +
> +		*info = (struct xfrm_tunnel_info) {
> +			.ifindex = t->dev->ifindex,
> +			.i_key = t->parms.i_key,
> +			.type = XFRM_TUNNEL_INFO_IP6,
> +			.ip6 = {
> +				.link = t->parms.link,
> +				.proto = t->parms.proto,
> +				.laddr = t->parms.laddr,
> +				.raddr = t->parms.raddr,
> +			},
> +		};
> +	}

[Severity: Low]
This isn't a bug, but can the if (t) branch be reached? Every in-tree
caller passes NULL. xfrm6_rcv() calls xfrm6_rcv_tnl(skb, NULL), and
xfrm6_tunnel.c and xfrm_interface_core.c pass NULL to xfrm6_rcv_spi().
That makes the branch, its kfree_skb() error path, and the new
<net/ip6_tunnel.h> include dead code.

Even if the branch were reached, xfrm_tunnel_check() and xfrm_input()
only read type and i_key for XFRM_TUNNEL_INFO_IP6. vti6_rcv_cb() only
accepts XFRM_TUNNEL_INFO_VTI6. The ifindex, link, proto, laddr and raddr
fields written here are never checked or used.

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111094629.230568-1-yaojun8558363%40gmail.com