linux-kernel - Re: [PATCH v1 1/1] xfrm: Use 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAHsH6Gv7m2Q=3sPZsRSMfqasay0P7j9zZT=DDwwgv7TuPX5agg@mail.gmail.com>
Date:   Sat, 2 Sep 2023 21:39:24 +0300
From:   Eyal Birger <eyal.birger@...il.com>
To:     Marcello Sylvester Bauer <email@....codeaurora.org>
Cc:     Steffen Klassert <steffen.klassert@...unet.com>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Marcello Sylvester Bauer <sylv@...v.io>,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 1/1] xfrm: Use skb_mac_header_was_set() to check for
 MAC header presence

Hi Marcello,

On Fri, Sep 1, 2023 at 7:15 PM Marcello Sylvester Bauer
<email@....codeaurora.org> wrote:
>
> From: Marcello Sylvester Bauer <sylv@...v.io>
>
> Replace skb->mac_len with skb_mac_header_was_set() in
> xfrm4_remove_tunnel_encap() and xfrm6_remove_tunnel_encap() to detect
> the presence of a MAC header. This change prevents a kernel page fault
> that could occur when a MAC address is added to an L3 interface using
> xfrm.
>
> Signed-off-by: Marcello Sylvester Bauer <sylv@...v.io>
> ---
>  net/xfrm/xfrm_input.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
> index d5ee96789d4b..afa1f1052065 100644
> --- a/net/xfrm/xfrm_input.c
> +++ b/net/xfrm/xfrm_input.c
> @@ -250,7 +250,7 @@ static int xfrm4_remove_tunnel_encap(struct xfrm_state *x, struct sk_buff *skb)
>
>         skb_reset_network_header(skb);
>         skb_mac_header_rebuild(skb);
> -       if (skb->mac_len)
> +       if (skb_mac_header_was_set(skb))
>                 eth_hdr(skb)->h_proto = skb->protocol;
>
>         err = 0;
> @@ -287,7 +287,7 @@ static int xfrm6_remove_tunnel_encap(struct xfrm_state *x, struct sk_buff *skb)
>
>         skb_reset_network_header(skb);
>         skb_mac_header_rebuild(skb);
> -       if (skb->mac_len)
> +       if (skb_mac_header_was_set(skb))
>                 eth_hdr(skb)->h_proto = skb->protocol;

I checked xfrm tunnels over both GRE and IPIP, and in both cases when reaching
this code the skb->mac_len was 0, whereas skb_mac_header_was_set()
was 1.

So in both cases this suggested patch would make this condition true and
write to an Ethernet header location on these L3 devices.

Can you please share your reproduction scenario for this case?

Thanks,
Eyal.

>
>         err = 0;
> --
> 2.42.0
>
>