netdev - Re: [PATCH ipsec] xfrm: Fix inner mode lookup in tunnel mode GSO segmentation

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d18ab53f-b91b-4c64-926f-4a1466d2d31e@nvidia.com>
Date: Mon, 17 Nov 2025 10:12:32 +0800
From: Jianbo Liu <jianbol@...dia.com>
To: Sabrina Dubroca <sd@...asysnail.net>
CC: <netdev@...r.kernel.org>, <davem@...emloft.net>, <kuba@...nel.org>,
	<steffen.klassert@...unet.com>, Herbert Xu <herbert@...dor.apana.org.au>,
	David Ahern <dsahern@...nel.org>, Eric Dumazet <edumazet@...gle.com>, "Paolo
 Abeni" <pabeni@...hat.com>, Simon Horman <horms@...nel.org>, Cosmin Ratiu
	<cratiu@...dia.com>
Subject: Re: [PATCH ipsec] xfrm: Fix inner mode lookup in tunnel mode GSO
 segmentation

On 11/17/2025 7:11 AM, Sabrina Dubroca wrote:
> 2025-11-14, 05:56:17 +0200, Jianbo Liu wrote:
>> Commit 61fafbee6cfe ("xfrm: Determine inner GSO type from packet
>> inner protocol") attempted to fix GSO segmentation by reading the
>> inner protocol from XFRM_MODE_SKB_CB(skb)->protocol. This was
>> incorrect as the XFRM_MODE_SKB_CB(skb)->protocol field is not assigned
>> a value in this code path and led to selecting the wrong inner mode.
> 
> Your testing didn't catch it before the patch was submitted? :(
> 

I admit I didn't test all the cases for the previous submission, but I 
have tested all the cases now with this fix.

> 
>> The correct value is in xfrm_offload(skb)->proto, which is set from
>> the outer tunnel header's protocol field by esp[4|6]_gso_encap(). It
>> is initialized by xfrm[4|6]_tunnel_encap_add() to either IPPROTO_IPIP
>> or IPPROTO_IPV6, using xfrm_af2proto() and correctly reflects the
>> inner packet's address family.
> 
> What's the call sequence that leads to calling
> xfrm4_tunnel_gso_segment without setting
> XFRM_MODE_SKB_CB(skb)->protocol? I'm seeing
> 
> xfrm_output -> xfrm_output2 -> xfrm_output_one
>   -> xfrm_outer_mode_output -> xfrm4_prepare_output
>   -> xfrm_inner_extract_output -> xfrm4_extract_output
> 
> (almost same as what ends up calling xfrm[4|6]_tunnel_encap_add)
> so XFRM_MODE_SKB_CB(skb)->protocol should be set?
> 

I think we both made mistaken.
a. XFRM_MODE_SKB_CB(skb)->protocol is assigned in that path, but it is 
assigned the value from ip_hdr(skb)->protocol. This means it holds the 
L4 protocol (e.g., IPPROTO_TCP or IPPROTO_UDP). However, to correctly 
determine the inner mode family, we need the tunnel protocols 
(IPPROTO_IPIP or IPPROTO_IPV6), which xfrm_af2proto() expects.

b. Furthermore, XFRM_MODE_SKB_CB(skb) shares the same memory layout as 
XFRM_SKB_CB(skb). This area can be overwritten during the transformation 
process (for example, in xfrm_replay_overflow and others), making the 
value in XFRM_MODE_SKB_CB unreliable by the time we reach GSO segmentation.

> 
> Also, after thinking about it more, I'm not so sure that
> xfrm_ip2inner_mode is wanted/needed in this context. Since we already
> have the inner protocol (whether it's via xo->proto or
> XFRM_MODE_SKB_CB(skb)->protocol), and all we care about is the inner
> family (to get the corresponding ethertype), we can just get it
> directly from the inner protocol without looking at
> x->inner_mode{,_iaf}? (pretty much just the reverse of xfrm_af2proto)
> 

I still prefer to reuse the logic in xfrm_af2proto()/xfrm_ip2inner_mode 
for two main reasons: a. It keeps the code easier to understand by using 
standard helpers rather than open-coding the reverse mapping. b. It 
keeps the logic directly related to the xfrm configuration and state 
properties.

Thanks!
Jianbo