[PATCH ipsec v2 0/2] xfrm: Correct inner packet family determination

[PATCH ipsec v2 0/2] xfrm: Correct inner packet family determination

[Jianbo Liu]

This series contains two 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.

V2:
 - The original first patch was sent separately to "ipsec-next"

Jianbo Liu (2):
  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/ipv6/esp6_offload.c | 6 ++++--
 net/xfrm/xfrm_device.c  | 2 +-
 net/xfrm/xfrm_output.c  | 2 +-
 4 files changed, 10 insertions(+), 6 deletions(-)

-- 
2.49.0

[PATCH ipsec v2 1/2] 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
unreliable because, for states handling both IPv4 and IPv6, the
relevant inner family could be either x->inner_mode.family or
x->inner_mode_iaf.family. Checking only the former can lead to a
mismatch with the actual packet being processed.

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.

Fixes: 91d8a53db219 ("xfrm: fix offloading of cross-family tunnels")
Fixes: 45a98ef4922d ("net/xfrm: IPsec tunnel mode fix inner_ipproto setting in sec_path")
Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
---
V2:
 - Change subject prefix, and send to "ipsec".
 - Update commit msg.
 - Add Fixes tag.

 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

[PATCH ipsec v2 2/2] 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.

Fixes: 26dbd66eab80 ("esp: choose the correct inner protocol for GSO on inter address family tunnels")
Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
---
V2:
 - Change subject prefix, and send to "ipsec".
 - Add Fixes tag.

 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

Re: [PATCH ipsec v2 2/2] xfrm: Determine inner GSO type from packet inner protocol

[Sabrina Dubroca]

2025-10-27, 04:40:59 +0200, Jianbo Liu wrote:
> 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.
> 
> Fixes: 26dbd66eab80 ("esp: choose the correct inner protocol for GSO on inter address family tunnels")
> Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
> Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
> ---
> V2:
>  - Change subject prefix, and send to "ipsec".
>  - Add Fixes tag.
> 
>  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);

I don't think this is correct. inner_mode_iaf is not always set by
__xfrm_init_state, only when we have a AF_UNSPEC selector, which is
not always the case for cross-family tunnels. So we would end up with
inner_mode->family = 0 here, and pass the wrong type to
skb_eth_gso_segment.

Other users of xfrm_ip2inner_mode (in ip_vti/ip6_vti, xfrmi) only call
it when we have an AF_UNSPEC selector, then we know inner_mode_iaf is
valid and can be used. Otherwise, the selector (ie x->inner_mode)
should have the right family for the packet (and all callers of
xfrm_ip2inner_mode use x->inner_mode when the selector is not
AF_UNSPEC).


Maybe it would be better to move the AF_UNSPEC check into
xfrm_ip2inner_mode, something like:

static inline const struct xfrm_mode *xfrm_ip2inner_mode(struct xfrm_state *x, int ipproto)
{
	if (x->sel.family != AF_UNSPEC)
		return &x->inner_mode;

	if ((ipproto == IPPROTO_IPIP && x->props.family == AF_INET) ||
	    (ipproto == IPPROTO_IPV6 && x->props.family == AF_INET6))
		return &x->inner_mode;
	else
		return &x->inner_mode_iaf;
}


since that's what all the callers are doing anyway?

Then it would be valid to use xfrm_ip2inner_mode like you're doing.

-- 
Sabrina

Re: [PATCH ipsec v2 2/2] xfrm: Determine inner GSO type from packet inner protocol

[Jianbo Liu]

On 10/27/2025 11:33 PM, Sabrina Dubroca wrote:
> 2025-10-27, 04:40:59 +0200, Jianbo Liu wrote:
>> 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.
>>
>> Fixes: 26dbd66eab80 ("esp: choose the correct inner protocol for GSO on inter address family tunnels")
>> Signed-off-by: Jianbo Liu <jianbol@nvidia.com>
>> Reviewed-by: Cosmin Ratiu <cratiu@nvidia.com>
>> ---
>> V2:
>>   - Change subject prefix, and send to "ipsec".
>>   - Add Fixes tag.
>>
>>   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);
> 
> I don't think this is correct. inner_mode_iaf is not always set by
> __xfrm_init_state, only when we have a AF_UNSPEC selector, which is
> not always the case for cross-family tunnels. So we would end up with
> inner_mode->family = 0 here, and pass the wrong type to
> skb_eth_gso_segment.
> 
> Other users of xfrm_ip2inner_mode (in ip_vti/ip6_vti, xfrmi) only call
> it when we have an AF_UNSPEC selector, then we know inner_mode_iaf is
> valid and can be used. Otherwise, the selector (ie x->inner_mode)
> should have the right family for the packet (and all callers of
> xfrm_ip2inner_mode use x->inner_mode when the selector is not
> AF_UNSPEC).
> 
> 
> Maybe it would be better to move the AF_UNSPEC check into
> xfrm_ip2inner_mode, something like:
> 
> static inline const struct xfrm_mode *xfrm_ip2inner_mode(struct xfrm_state *x, int ipproto)
> {
> 	if (x->sel.family != AF_UNSPEC)
> 		return &x->inner_mode;
> 
> 	if ((ipproto == IPPROTO_IPIP && x->props.family == AF_INET) ||
> 	    (ipproto == IPPROTO_IPV6 && x->props.family == AF_INET6))
> 		return &x->inner_mode;
> 	else
> 		return &x->inner_mode_iaf;
> }
> 
> 
> since that's what all the callers are doing anyway?
> 
> Then it would be valid to use xfrm_ip2inner_mode like you're doing.

It makes sense. I will submit v3 soon. Thanks!

>