netdev - Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not null entry.

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:   Fri, 21 Jul 2017 14:53:21 -0700
From:   Roopa Prabhu <roopa@...ulusnetworks.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     Hangbin Liu <liuhangbin@...il.com>,
        network dev <netdev@...r.kernel.org>
Subject: Re: [PATCH net] ipv6: no need to return rt->dst.error if it is not
 null entry.

On Fri, Jul 21, 2017 at 11:42 AM, Cong Wang <xiyou.wangcong@...il.com> wrote:
> On Thu, Jul 20, 2017 at 8:23 AM, Hangbin Liu <liuhangbin@...il.com> wrote:
>> 2017-07-20 23:06 GMT+08:00 Hangbin Liu <liuhangbin@...il.com>:
>>>> +++ b/net/ipv6/route.c
>>>> @@ -3637,12 +3637,6 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
>>>>                 dst = ip6_route_lookup(net, &fl6, 0);
>>>>
>>>>         rt = container_of(dst, struct rt6_info, dst);
>>>> -       if (rt->dst.error) {
>>>> -               err = rt->dst.error;
>>>> -               ip6_rt_put(rt);
>>>> -               goto errout;
>>>> -       }
>>>
>>> hmm... or instead of remove this check, should we check all the entry? Like
>>> if ((rt->dst.error && rt != net->ipv6.ip6_null_entry && rt !=
>>                                                     ^^ mistake here
>>> net->ipv6.ip6_blk_hole_entry) ||
>>>      rt == net->ipv6.ip6_null_entry )
>>
>> Sorry,  there should be no need to check ip6_null_entry since the
>> error is already
>> -ENETUNREACH. So how about
>
> Hmm? All of these 3 entries have error set, right??
> So we should only check dst.error...

removing the check seems ok to me. We can also make the check
conditional to fibmatch code only
to eliminate any change in behavior introduced by fibmatch.

ie if (fibmatch && rt->dst.error).

Hangbin, can you also pls check that fibmatch works ok for such routes
with your patch applied ?.

ip netns exec client ip -6 route get fibmatch 2003::1     (with latest iproute2)

thank you.