netdev - Re: [PATCH] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128 local route (and others)

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAKD1Yr2tcRiiLwGdTB3TwpxoAH0+R=dgfCDh6TpZ2fHTE2rC9w@mail.gmail.com>
Date:   Mon, 2 Sep 2019 11:12:35 +0900
From:   Lorenzo Colitti <lorenzo@...gle.com>
To:     Maciej Żenczykowski <zenczykowski@...il.com>
Cc:     Maciej Żenczykowski <maze@...gle.com>,
        "David S . Miller" <davem@...emloft.net>,
        Linux NetDev <netdev@...r.kernel.org>,
        David Ahern <dsahern@...il.com>
Subject: Re: [PATCH] net-ipv6: fix excessive RTF_ADDRCONF flag on ::1/128
 local route (and others)

On Mon, Sep 2, 2019 at 2:55 AM Maciej Żenczykowski
<zenczykowski@...il.com> wrote:
> It's not immediately clear to me what is the better approach as I'm
> not immediately certain what RTF_ADDRCONF truly means.
> However the in kernel header file comment does explicitly mention this
> being used to flag routes derived from RA's, and very clearly ::1/128
> is not RA generated, so I *think* the correct fix is to return to the
> old way the kernel used to do things and not flag with ADDRCONF...

AIUI, "addrconf" has always meant stateless address autoconfiguration
as per RFC 4862, i.e., addresses autoconfigured when getting an RA, or
autoconfigured based on adding the link-local prefix. Looking at 5.1
(the most recent release before c7a1ce397ada which you're fixing here)
confirms this interpretation, because RTF_ADDRCONF is only used by:

- addrconf_prefix_rcv: receiving a PIO from an RA
- rt6_add_route_info: receiving an RIO from an RA
- rt6_add_dflt_router, rt6_get_dflt_router: receiving the default
router from an RA
- __rt6_purge_dflt_routers: removing all routes received from RAs,
when enabling forwarding (i.e., switching from being a host to being a
router)

So, if I'm reading c7a1ce397ada right, I would say it's incorrect.
That patch changes things so that RTF_ADDRCONF is set for pretty much
all routes created by adding IPv6 addresses. That includes not only
IPv6 addresses created by RAs, which has always been the case, but
also IPv6 addresses created manually from userspace, or the loopback
address, and even multicast and anycast addresses created by
IPV6_JOIN_GROUP and IPV6_JOIN_ANYCAST. That's userspace-visible
breakage and should be reverted.

Not sure if this patch is the right fix, though, because it breaks
things in the opposite direction: even routes created by an IPv6
address added by receiving an RA will no longer have RTF_ADDRCONF.
Perhaps add something like this as well?

 struct fib6_info *addrconf_f6i_alloc(struct net *net, struct inet6_dev *idev,
-                                     const struct in6_addr *addr, bool anycast,
-                                     const struct in6_addr *addr, u8 flags,
                                      gfp_t gfp_flags);

flags would be RTF_ANYCAST iff the code previously called with true,
and RTF_ADDRCONF if called by a function that is adding an IPv6
address coming from an RA.