netdev - Re: [PATCH net] ip6_tunnel: set inner ipproto before ip6_tnl

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]

Date:   Thu, 29 Oct 2020 10:40:37 -0400
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Alexander Ovechkin <ovov@...dex-team.ru>
Cc:     Willem de Bruijn <willemdebruijn.kernel@...il.com>,
        vfedorenko@...ek.ru, Network Development <netdev@...r.kernel.org>,
        Tom Herbert <tom@...bertland.com>
Subject: Re: [PATCH net] ip6_tunnel: set inner ipproto before ip6_tnl_encap.

On Thu, Oct 29, 2020 at 3:46 AM Alexander Ovechkin <ovov@...dex-team.ru> wrote:
>
> On 28 Oct 2020, at 01:53 UTC Willem de Bruijn <willemdebruijn.kernel@...il.com> wrote:
> > On Tue, Oct 27, 2020 at 5:52 PM Alexander Ovechkin <ovov@...dex-team.ru> wrote:
> > >
> > > > But it was moved on purpose to avoid setting the inner protocol to IPPROTO_MPLS. That needs to use skb->inner_protocol to further segment.
> > > And why do we need to avoid setting the inner protocol to IPPROTO_MPLS? Currently skb->inner_protocol is used before call of ip6_tnl_xmit.
> > > Can you please give example when this patch breaks MPLS segmentation?
> >
> > mpls_gso_segment calls skb_mac_gso_segment on the inner packet. After
> > setting skb->protocol based on skb->inner_protocol.
>
> Yeah, but mpls_gso_segment is called before ip6_tnl_xmit (because tun devices don't have NETIF_F_GSO_SOFTWARE in their mpls_features), so it does not matter to what value ip6_tnl_xmit sets skb->inner_ipproto.
> And even if gso would been called after both mpls_xmit and ip6_tnl_xmit it would fail as you have written.

Good point. Okay, if no mpls gso packets can make it here, then it
should not matter.

Vadim, are we missing another reason for this move?

Else, no other concerns from me. Please do add a Fixes tag.