[PATCH net-next 0/3] xfrm: Correct inner packet family determination

[PATCH net-next 0/3] xfrm: Correct inner packet family determination

[Jianbo Liu]

This series contains three patches addressing issues in the XFRM
subsystem where the code incorrectly relied on static family fields
from the xfrm_state instead of determining the family from the actual
packet being processed. 

This was particularly problematic in tunnel mode scenarios when using
states that could handle different inner families.

Jianbo Liu (3):
  xfrm: Remove redundant state inner mode check
  xfrm: Check inner packet family directly from skb_dst
  xfrm: Determine inner GSO type from packet inner protocol

 net/ipv4/esp4_offload.c        | 6 ++++--
 net/ipv4/ip_vti.c              | 8 +-------
 net/ipv6/esp6_offload.c        | 6 ++++--
 net/ipv6/ip6_vti.c             | 8 +-------
 net/xfrm/xfrm_device.c         | 2 +-
 net/xfrm/xfrm_interface_core.c | 8 +-------
 net/xfrm/xfrm_output.c         | 2 +-
 net/xfrm/xfrm_policy.c         | 9 ++-------
 8 files changed, 15 insertions(+), 34 deletions(-)

-- 
2.49.0

[PATCH net-next 1/3] xfrm: Remove redundant state inner mode check

[Jianbo Liu]

This check is redundant because xfrm_ip2inner_mode no longer returns
NULL, because of the change in commit c9500d7b7de8 ("xfrm: store
xfrm_mode directly, not its address").

Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
---
 net/ipv4/ip_vti.c              | 8 +-------
 net/ipv6/ip6_vti.c             | 8 +-------
 net/xfrm/xfrm_interface_core.c | 8 +-------
 net/xfrm/xfrm_policy.c         | 9 ++-------
 4 files changed, 5 insertions(+), 28 deletions(-)

diff --git a/net/ipv4/ip_vti.c b/net/ipv4/ip_vti.c
index 95b6bb78fcd2..91e889965918 100644
--- a/net/ipv4/ip_vti.c
+++ b/net/ipv4/ip_vti.c
@@ -120,14 +120,8 @@ static int vti_rcv_cb(struct sk_buff *skb, int err)
 
 	inner_mode = &x->inner_mode;
 
-	if (x->sel.family == AF_UNSPEC) {
+	if (x->sel.family == AF_UNSPEC)
 		inner_mode = xfrm_ip2inner_mode(x, XFRM_MODE_SKB_CB(skb)->protocol);
-		if (inner_mode == NULL) {
-			XFRM_INC_STATS(dev_net(skb->dev),
-				       LINUX_MIB_XFRMINSTATEMODEERROR);
-			return -EINVAL;
-		}
-	}
 
 	family = inner_mode->family;
 
diff --git a/net/ipv6/ip6_vti.c b/net/ipv6/ip6_vti.c
index ad5290be4dd6..67030918279c 100644
--- a/net/ipv6/ip6_vti.c
+++ b/net/ipv6/ip6_vti.c
@@ -364,14 +364,8 @@ static int vti6_rcv_cb(struct sk_buff *skb, int err)
 
 	inner_mode = &x->inner_mode;
 
-	if (x->sel.family == AF_UNSPEC) {
+	if (x->sel.family == AF_UNSPEC)
 		inner_mode = xfrm_ip2inner_mode(x, XFRM_MODE_SKB_CB(skb)->protocol);
-		if (inner_mode == NULL) {
-			XFRM_INC_STATS(dev_net(skb->dev),
-				       LINUX_MIB_XFRMINSTATEMODEERROR);
-			return -EINVAL;
-		}
-	}
 
 	family = inner_mode->family;
 
diff --git a/net/xfrm/xfrm_interface_core.c b/net/xfrm/xfrm_interface_core.c
index 330a05286a56..7084c7e6cf7a 100644
--- a/net/xfrm/xfrm_interface_core.c
+++ b/net/xfrm/xfrm_interface_core.c
@@ -389,14 +389,8 @@ static int xfrmi_rcv_cb(struct sk_buff *skb, int err)
 	if (xnet) {
 		inner_mode = &x->inner_mode;
 
-		if (x->sel.family == AF_UNSPEC) {
+		if (x->sel.family == AF_UNSPEC)
 			inner_mode = xfrm_ip2inner_mode(x, XFRM_MODE_SKB_CB(skb)->protocol);
-			if (inner_mode == NULL) {
-				XFRM_INC_STATS(dev_net(skb->dev),
-					       LINUX_MIB_XFRMINSTATEMODEERROR);
-				return -EINVAL;
-			}
-		}
 
 		if (!xfrm_policy_check(NULL, XFRM_POLICY_IN, skb,
 				       inner_mode->family))
diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index 62486f866975..a1d995db6293 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -2711,15 +2711,10 @@ static struct dst_entry *xfrm_bundle_create(struct xfrm_policy *policy,
 			 */
 			xfrm_dst_set_child(xdst_prev, &xdst->u.dst);
 
-		if (xfrm[i]->sel.family == AF_UNSPEC) {
+		if (xfrm[i]->sel.family == AF_UNSPEC)
 			inner_mode = xfrm_ip2inner_mode(xfrm[i],
 							xfrm_af2proto(family));
-			if (!inner_mode) {
-				err = -EAFNOSUPPORT;
-				dst_release(dst);
-				goto put_states;
-			}
-		} else
+		else
 			inner_mode = &xfrm[i]->inner_mode;
 
 		xdst->route = dst;
-- 
2.49.0

[PATCH net-next 2/3] xfrm: Check inner packet family directly from skb_dst

[Jianbo Liu]

In the output path, xfrm_dev_offload_ok and xfrm_get_inner_ipproto
need to determine the protocol family of the inner packet (skb) before
it gets encapsulated.

In xfrm_dev_offload_ok, the code checked x->inner_mode.family. This is
incorrect because the state's true inner family might be specified in
x->inner_mode_iaf.family (e.g., for tunnel mode).

In xfrm_get_inner_ipproto, the code checked x->outer_mode.family. This
is also incorrect for tunnel mode, as the inner packet's family can be
different from the outer header's family.

At both of these call sites, the skb variable holds the original inner
packet. The most direct and reliable source of truth for its protocol
family is its destination entry. This patch fixes the issue by using
skb_dst(skb)->ops->family to ensure protocol-specific headers are only
accessed for the correct packet type.

Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
---
 net/xfrm/xfrm_device.c | 2 +-
 net/xfrm/xfrm_output.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/xfrm/xfrm_device.c b/net/xfrm/xfrm_device.c
index 44b9de6e4e77..52ae0e034d29 100644
--- a/net/xfrm/xfrm_device.c
+++ b/net/xfrm/xfrm_device.c
@@ -438,7 +438,7 @@ bool xfrm_dev_offload_ok(struct sk_buff *skb, struct xfrm_state *x)
 
 	check_tunnel_size = x->xso.type == XFRM_DEV_OFFLOAD_PACKET &&
 			    x->props.mode == XFRM_MODE_TUNNEL;
-	switch (x->inner_mode.family) {
+	switch (skb_dst(skb)->ops->family) {
 	case AF_INET:
 		/* Check for IPv4 options */
 		if (ip_hdr(skb)->ihl != 5)
diff --git a/net/xfrm/xfrm_output.c b/net/xfrm/xfrm_output.c
index 9077730ff7d0..a98b5bf55ac3 100644
--- a/net/xfrm/xfrm_output.c
+++ b/net/xfrm/xfrm_output.c
@@ -698,7 +698,7 @@ static void xfrm_get_inner_ipproto(struct sk_buff *skb, struct xfrm_state *x)
 		return;
 
 	if (x->outer_mode.encap == XFRM_MODE_TUNNEL) {
-		switch (x->outer_mode.family) {
+		switch (skb_dst(skb)->ops->family) {
 		case AF_INET:
 			xo->inner_ipproto = ip_hdr(skb)->protocol;
 			break;
-- 
2.49.0

Re: [PATCH net-next 2/3] xfrm: Check inner packet family directly from skb_dst

[Sabrina Dubroca]

Some general notes:

 - ipsec patches should go through the ipsec/ipsec-next trees
   maintained by Steffen, not net/net-next directly, and use
   ipsec/ipsec-next in the subject prefix

 - this patch looks more like a bugfix, so it should target the ipsec
   tree and have a Fixes tag, instead of -next


2025-10-24, 05:31:36 +0300, Jianbo Liu wrote:
> In the output path, xfrm_dev_offload_ok and xfrm_get_inner_ipproto
> need to determine the protocol family of the inner packet (skb) before
> it gets encapsulated.
> 
> In xfrm_dev_offload_ok, the code checked x->inner_mode.family. This is
> incorrect because the state's true inner family might be specified in
> x->inner_mode_iaf.family (e.g., for tunnel mode).

I wouldn't say inner_mode_iaf.family is the "true" inner family. AFAIU
it's the other possible inner family for states that accept both
(I got that wrong in 91d8a53db219 ("xfrm: fix offloading of
cross-family tunnels")).

> In xfrm_get_inner_ipproto, the code checked x->outer_mode.family. This
> is also incorrect for tunnel mode, as the inner packet's family can be
> different from the outer header's family.
> 
> At both of these call sites, the skb variable holds the original inner
> packet. The most direct and reliable source of truth for its protocol
> family is its destination entry.

(the IP version would also work since it's in the same place for both
v4 and v6, but we have other uses of dst family in xfrm_output so it
should be fine)

> This patch fixes the issue by using
> skb_dst(skb)->ops->family to ensure protocol-specific headers are only
> accessed for the correct packet type.

Do you have an example of problematic setup? I didn't run into that
when I wrote 91d8a53db219.

-- 
Sabrina

Re: [PATCH net-next 2/3] xfrm: Check inner packet family directly from skb_dst

[Jianbo Liu]

On 10/24/2025 7:26 PM, Sabrina Dubroca wrote:
> Some general notes:
> 
>   - ipsec patches should go through the ipsec/ipsec-next trees
>     maintained by Steffen, not net/net-next directly, and use
>     ipsec/ipsec-next in the subject prefix
> 
>   - this patch looks more like a bugfix, so it should target the ipsec
>     tree and have a Fixes tag, instead of -next
> 

Thanks for pointing that. I'll send a v2 addressing these:

Patch 1/3 will be sent separately to the ipsec-next tree.

Patches 2/3 will target the ipsec tree and include appropriate Fixes tags.

> 
> 2025-10-24, 05:31:36 +0300, Jianbo Liu wrote:
>> In the output path, xfrm_dev_offload_ok and xfrm_get_inner_ipproto
>> need to determine the protocol family of the inner packet (skb) before
>> it gets encapsulated.
>>
>> In xfrm_dev_offload_ok, the code checked x->inner_mode.family. This is
>> incorrect because the state's true inner family might be specified in
>> x->inner_mode_iaf.family (e.g., for tunnel mode).
> 
> I wouldn't say inner_mode_iaf.family is the "true" inner family. AFAIU
> it's the other possible inner family for states that accept both
> (I got that wrong in 91d8a53db219 ("xfrm: fix offloading of
> cross-family tunnels")).

You're right, my wording wasn't precise. My intention was to highlight 
that relying only on inner_mode_iaf.family is insufficient. I'll correct 
the commit message in v2 to reflect this accurately.

> 
>> In xfrm_get_inner_ipproto, the code checked x->outer_mode.family. This
>> is also incorrect for tunnel mode, as the inner packet's family can be
>> different from the outer header's family.
>>
>> At both of these call sites, the skb variable holds the original inner
>> packet. The most direct and reliable source of truth for its protocol
>> family is its destination entry.
> 
> (the IP version would also work since it's in the same place for both
> v4 and v6, but we have other uses of dst family in xfrm_output so it
> should be fine)

My initial version did check the IP version field directly. I changed it 
because I noticed skb_dst being used in other parts of xfrm_output.c and 
aimed for consistency, but I agree either approach works here.

> 
>> This patch fixes the issue by using
>> skb_dst(skb)->ops->family to ensure protocol-specific headers are only
>> accessed for the correct packet type.
> 
> Do you have an example of problematic setup? I didn't run into that
> when I wrote 91d8a53db219.

The issue was found during standard validation testing. There wasn't a 
complex configuration involved, simply setting up tunnel mode 
connections where the inner and outer protocol families differed (e.g., 
IPv4-in-IPv6 or vice-versa).

> 

[PATCH net-next 3/3] xfrm: Determine inner GSO type from packet inner protocol

[Jianbo Liu]

The GSO segmentation functions for ESP tunnel mode
(xfrm4_tunnel_gso_segment and xfrm6_tunnel_gso_segment) were
determining the inner packet's L2 protocol type by checking the static
x->inner_mode.family field from the xfrm state.

This is unreliable. In tunnel mode, the state's actual inner family
could be defined by x->inner_mode.family or by
x->inner_mode_iaf.family. Checking only the former can lead to a
mismatch with the actual packet being processed, causing GSO to create
segments with the wrong L2 header type.

This patch fixes the bug by deriving the inner mode directly from the
packet's inner protocol stored in XFRM_MODE_SKB_CB(skb)->protocol.

Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
---
 net/ipv4/esp4_offload.c | 6 ++++--
 net/ipv6/esp6_offload.c | 6 ++++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/net/ipv4/esp4_offload.c b/net/ipv4/esp4_offload.c
index e0d94270da28..05828d4cb6cd 100644
--- a/net/ipv4/esp4_offload.c
+++ b/net/ipv4/esp4_offload.c
@@ -122,8 +122,10 @@ static struct sk_buff *xfrm4_tunnel_gso_segment(struct xfrm_state *x,
 						struct sk_buff *skb,
 						netdev_features_t features)
 {
-	__be16 type = x->inner_mode.family == AF_INET6 ? htons(ETH_P_IPV6)
-						       : htons(ETH_P_IP);
+	const struct xfrm_mode *inner_mode = xfrm_ip2inner_mode(x,
+					XFRM_MODE_SKB_CB(skb)->protocol);
+	__be16 type = inner_mode->family == AF_INET6 ? htons(ETH_P_IPV6)
+						     : htons(ETH_P_IP);
 
 	return skb_eth_gso_segment(skb, features, type);
 }
diff --git a/net/ipv6/esp6_offload.c b/net/ipv6/esp6_offload.c
index 7b41fb4f00b5..22410243ebe8 100644
--- a/net/ipv6/esp6_offload.c
+++ b/net/ipv6/esp6_offload.c
@@ -158,8 +158,10 @@ static struct sk_buff *xfrm6_tunnel_gso_segment(struct xfrm_state *x,
 						struct sk_buff *skb,
 						netdev_features_t features)
 {
-	__be16 type = x->inner_mode.family == AF_INET ? htons(ETH_P_IP)
-						      : htons(ETH_P_IPV6);
+	const struct xfrm_mode *inner_mode = xfrm_ip2inner_mode(x,
+					XFRM_MODE_SKB_CB(skb)->protocol);
+	__be16 type = inner_mode->family == AF_INET ? htons(ETH_P_IP)
+						    : htons(ETH_P_IPV6);
 
 	return skb_eth_gso_segment(skb, features, type);
 }
-- 
2.49.0