netdev - Re: [PATCH ipsec v2] xfrm: check MAC header is shown with both skb->mac_len and skb_mac_header_was

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: <CANn89iL-r3+HBC10m+QdFVn20DdNH=r5EBQDV=EmewWm6Vsyqg@mail.gmail.com>
Date: Tue, 5 Nov 2024 10:25:35 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: En-Wei WU <en-wei.wu@...onical.com>
Cc: Peter Seiderer <ps.report@....net>, steffen.klassert@...unet.com, 
	herbert@...dor.apana.org.au, davem@...emloft.net, kuba@...nel.org, 
	pabeni@...hat.com, netdev@...r.kernel.org, linux-kernel@...r.kernel.org, 
	kai.heng.feng@...onical.com, chia-lin.kao@...onical.com, 
	anthony.wong@...onical.com, kuan-ying.lee@...onical.com, 
	chris.chiu@...onical.com
Subject: Re: [PATCH ipsec v2] xfrm: check MAC header is shown with both
 skb->mac_len and skb_mac_header_was_set()

On Fri, Oct 18, 2024 at 3:22 PM En-Wei WU <en-wei.wu@...onical.com> wrote:
>
> > Seems like the __netif_receive_skb_core() and dev_gro_receive() are
> > the places where it calls skb_reset_mac_len() with skb->mac_header =
> > ~0U.
> I believe it's the root cause.
>
> My concern is that if we put something like:
> +       if (!skb_mac_header_was_set(skb)) {
> +               DEBUG_NET_WARN_ON_ONCE(1);
> +               skb->mac_len = 0;
> in skb_reset_mac_len(), it may degrade the RX path a bit.

I do not have such concerns. Note this is temporary until we fix the root cause.

>
> Catching the bug in xfrm4_remove_tunnel_encap() and
> xfrm6_remove_tunnel_encap() (the original patch) is nice because it
> won't affect the systems which are not using the xfrm.
>

Somehow xfrm is feeding to gro_cells_receive() packets without the mac
header being set, this is the bug that needs to be fixed.

GRO needs skb_mac_header() to return the correct pointer.

For normal GRO, it is set either in :

1) napi_gro_frags : napi_frags_skb()  calls skb_reset_mac_header(skb);

2) napi_gro_receive() : callers are supposed to call eth_type_trans()
before calling napi_gro_receive().
    eth_type_trans() calls skb_reset_mac_header() as expected.

xfrm calls skb_mac_header_rebuild(), but it might be a NOP if MAC
header was never set.