Re: [PATCH v1 1/1] xfrm: Use skb_mac_header_was_set() to check for MAC header presence

[Eyal Birger]

Hi Marcello,

On Fri, Sep 1, 2023 at 7:15 PM Marcello Sylvester Bauer
<email@xxxxxxxxxxxxxxxxxx> wrote:
>
> From: Marcello Sylvester Bauer <sylv@xxxxxxx>
>
> 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@xxxxxxx>
> ---
>  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
>
>