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:   Sat, 22 Jul 2017 21:54:38 -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 2:53 PM, Roopa Prabhu <roopa@...ulusnetworks.com> wrote:
> 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.

I tried this with your patch and this works for the fibmatch case as well.