netdev - Re: [PATCH iproute/master 2/3] iptunnel: add support for mpls/ip to sit tunnels

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: <20170615183117.GA2704@templeofstupid.com>
Date:   Thu, 15 Jun 2017 11:31:18 -0700
From:   Krister Johansen <kjlx@...pleofstupid.com>
To:     David Ahern <dsahern@...il.com>
Cc:     Krister Johansen <kjlx@...pleofstupid.com>,
        Stephen Hemminger <stephen@...workplumber.org>,
        netdev@...r.kernel.org, simon.horman@...ronome.com
Subject: Re: [PATCH iproute/master 2/3] iptunnel: add support for mpls/ip to
 sit tunnels

On Wed, Jun 14, 2017 at 11:16:20AM -0600, David Ahern wrote:
> On 6/14/17 11:11 AM, Krister Johansen wrote:
> > I did try to fix this up as part of bringing this patch up to date,
> > since it was one of the concerns that David raised too.  I believe the
> > problem that I ran into was that IPPROTO_MPLS wasn't defined in all
> > versions of the headers where I tried to include them, and by bringing
> > in in.h, I also managed to get a bunch of errors around re-definition of
> > other symbols.
> > 
> > That said, I don't believe that 137 as the IPPROTO_MPLS value should
> > change anytime soon.  It's defined in RFC 4023.
> > 
> > 	https://tools.ietf.org/html/rfc4023
> > 
> > However, if this is still seems problematic, I can take another shot at
> > attempting to clean this up further.
> 
> IPPROTO_MPLS is defined in include/linux/in.h in iproute2.

Right.  I poked at this some more yesterday afternoon.

The IPPROTO enum in include/linux/in.h is wrapped in an #if
__UAPI_DEF_IN_IPPROTO.  That's defined in include/linux/libc-compat.h.
If _NETINET_IN_H is defined, then __UAPI_DEF_IN_IPPROTO is undefined.
In that case, we pick up the the IPPROTO defs from netinet/in.h.  The
addition of IPPROTO_MPLS to utils.h is to cover the case where we're
using a system netinet/in.h that's older and doesn't yet have an
IPPROTO_MPLS defintion.  There seems to be prescedent for this, since
utils.h also includes IPPROTO_ESP, IPPROTO_AH, and IPPROTO_COMP.

I spent some time trying to remove includes of netinet/in.h from the
code in this patch, however, it's also included by other system headers
that are included throughout iproute2.  I'm not saying removing this is
impossible, but it certainly seems like it's beyond the scope of this
particular patch.

-K