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

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] [day] [month] [year] [list]

Message-ID: <c71f734b-d215-497c-bd65-dfb7ab3159ee@nvidia.com>
Date: Mon, 27 Oct 2025 09:50:01 +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>, Cosmin Ratiu <cratiu@...dia.com>, Herbert Xu
	<herbert@...dor.apana.org.au>, Eric Dumazet <edumazet@...gle.com>, "Paolo
 Abeni" <pabeni@...hat.com>, Simon Horman <horms@...nel.org>
Subject: Re: [PATCH net-next 2/3] xfrm: Check inner packet family directly
 from skb_dst



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).

>