netdev - Re: [Patch net] ipv4: fib: check forwarding before checking send

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: <D857DE45-97A1-45C1-B88E-F3657DCC3E33@gmail.com>
Date:	Sat, 12 Apr 2014 13:32:03 +0800
From:	Rex Ge <lungothrin@...il.com>
To:	Cong Wang <xiyou.wangcong@...il.com>
Cc:	Julian Anastasov <ja@....bg>, David Miller <davem@...emloft.net>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	Cong Wang <cwang@...pensource.com>
Subject: Re: [Patch net] ipv4: fib: check forwarding before checking send_redirects

sorry for send this the third time, I believe I fix the Content-Type this time.


I agree with David.
If your packet fail RP filter, that means something else went seriously wrong. This is not the fix.
some information, like what is the "src addr" you refer to, what does your routing table look like, would help

Rex

On Apr 12, 2014, at 3:11 AM, Cong Wang <xiyou.wangcong@...il.com> wrote:

> On Wed, Apr 9, 2014 at 1:03 AM, Julian Anastasov <ja@....bg> wrote:
>> 
>>        Hello,
>> 
>> On Tue, 8 Apr 2014, Cong Wang wrote:
>> 
>>>>        Yes, the code will fail if input route is
>>>> tried on loopback device. My first thought was that
>>>> you forgot to 'ip route flush cache' after clearing
>>>> rp_filter.
>>>> 
>>> 
>>> route cache is removed after 3.6, so I don't need to do it?
>> 
>>        route cache is now FIB cache :)
>> 
>>        My 'ip' tool still does
>> 
>> open("/proc/sys/net/ipv4/route/flush", O_WRONLY) = 4
>> write(4, "-1", 2) = 2
>> close(4)          = 0
>> 
>>        Which invalidates the FIB cache:
>> ipv4_sysctl_rtcache_flush -> rt_cache_flush, fnhe_genid_bump
>> 
>>        The change is noticed via rt_cache_valid and
>> rt_is_expired.
> 
> OK, I will add this.
> 
>> 
>>>>        What kind of packet is that and why the skb lost its
>>>> output route and now it is trying for input route?
>>>> 
>>> 
>>> Yeah, I just noticed that for normal loopback packets its ->dst
>>> are forced to be kept so that it will not need to lookup route
>>> again on rx. However, in our case, we redirect the packets
>>> from veth0 to lo when src_addr == dst_addr, its ->dst is dropped
>>> after coming out of the network namespace, therefore goes into
>>> the slow path to validate the reverse path and lookup the route.
>> 
>>        I'm still not sure that it is valid to process
>> input route on loopback device. There is also a case
>> of broadcasts/multicasts that should preserve their local output
>> route while being looped in ip_mc_output(). In this
>> case device is not loopback. I need more time to check
>> the code again...
> 
> OK, please do, you are the expert on routing. :)
> 
>> 
>>> I think David's point is still valid in this case, __fib_validate_source()
>>> should not reject the packets. So hmm... the only reason I can find is
>>> the following code:
>>> 
>>>        if (fib_lookup(net, &fl4, &res) == 0) {
>>>                if (res.type == RTN_UNICAST)
>>>                        ret = FIB_RES_NH(res).nh_scope >= RT_SCOPE_HOST;
>>>        }
>> 
>>        I think fib_validate_source is called with
>> oif=LOOPBACK_IFINDEX and dev/idev=lo for skb without route,
>> that is the problem. In this case fib_validate_source
>> thinks this is forwarding from lo to lo. So, there is
>> a reason your patch fixes the problem, alternative
>> is to add oif == LOOPBACK_IFINDEX check:
>> 
>>        dev->ifindex != oif || !IN_DEV_TX_REDIRECTS(idev) ||
>>        oif == LOOPBACK_IFINDEX
> 
> Yeah, actually this is my initial fix, but I thought making an
> exception here for loopback does not look good therefore dropped
> this idea.
> 
>> 
>>> where its nh_scope == RT_SCOPE_HOST?
>> 
>>        The idea is to avoid calling __fib_validate_source,
>> right? No need to look into __fib_validate_source. Also,
>> I'm not sure how legal is to receive packets on lo without
>> skb->dst...
>> 
> 
> That's my point, but David seems not agree. :)
> 
> Thanks!
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@...r.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html