netdev - Re: [Patch net] ipv6: ignore null_entry in inet6_rtm

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: <e4c27c86-b97a-e37b-c2a3-8329f1a7301c@cumulusnetworks.com>
Date:   Tue, 28 Feb 2017 14:35:22 -0800
From:   David Ahern <dsa@...ulusnetworks.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     Linux Kernel Network Developers <netdev@...r.kernel.org>,
        Andrey Konovalov <andreyknvl@...gle.com>
Subject: Re: [Patch net] ipv6: ignore null_entry in inet6_rtm_getroute() too

On 2/28/17 11:48 AM, Cong Wang wrote:
> On Tue, Feb 28, 2017 at 11:01 AM, David Ahern <dsa@...ulusnetworks.com> wrote:
>> On 2/28/17 10:44 AM, Cong Wang wrote:
>>> Like commit 1f17e2f2c8a8 ("net: ipv6: ignore null_entry on route dumps"),
>>> we need to ignore null entry in inet6_rtm_getroute() too.
>>>
>>> Return -ENOENT here because we return the same errno when deleting
>>> the null entry.
>>>
>>> Fixes: a1a22c1206 ("net: ipv6: Keep nexthop of multipath route on admin down")
>>> Reported-by: Dmitry Vyukov <dvyukov@...gle.com>
>>> Cc: David Ahern <dsa@...ulusnetworks.com>
>>> Signed-off-by: Cong Wang <xiyou.wangcong@...il.com>
>>> ---
>>>  net/ipv6/route.c | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>>> index f54f426..25590d1 100644
>>> --- a/net/ipv6/route.c
>>> +++ b/net/ipv6/route.c
>>> @@ -3627,6 +3627,12 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh)
>>>               rt = (struct rt6_info *)ip6_route_output(net, NULL, &fl6);
>>>       }
>>>
>>> +     if (rt == net->ipv6.ip6_null_entry) {
>>> +             ip6_rt_put(rt);
>>> +             err = -ENOENT;
>>> +             goto errout;
>>> +     }
>>> +
>>>       skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
>>>       if (!skb) {
>>>               ip6_rt_put(rt);
>>>
>>
>> hold on. That test exposed something else, not just a getroute problem.
>> I accidentally ran 'unsahre -n; ip -6 ro ls' on my host machine instead
>> of a VM, so took some time to recover. dumproute already covers the null
>> route.

My host was running a slightly older kernel (did not have the null_entry
check in the dump route path for one).

As for trapping null_entry on getroute, this changes user experience.
Right now you always get a route response for IPv6 with the error set as
rta_error. This patch changes that. I am fine with it since it makes
IPv6 more like IPv4:

# ip -6 ro get 2001:db8:12::1
RTNETLINK answers: Network is unreachable

But, if we are going to do this then err should be set based on
rt->dst.error (ENOENT is not the right error) and the commit message
should state the change.