netdev - Re: [PATCHv5 net-next] ipv6: do not match device when remove source route

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: <4cb32d96-6db8-fd4a-be18-52b4526a45b1@kernel.org>
Date: Mon, 14 Aug 2023 10:13:26 -0600
From: David Ahern <dsahern@...nel.org>
To: Hangbin Liu <liuhangbin@...il.com>, Ido Schimmel <idosch@...sch.org>
Cc: netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Thomas Haller <thaller@...hat.com>
Subject: Re: [PATCHv5 net-next] ipv6: do not match device when remove source
 route

On 8/14/23 2:33 AM, Hangbin Liu wrote:
> Hi Ido,
> On Sun, Aug 13, 2023 at 07:09:46PM +0300, Ido Schimmel wrote:
>> On Fri, Aug 11, 2023 at 05:53:08PM +0800, Hangbin Liu wrote:
>>> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
>>> index 64e873f5895f..0f981cc5bed1 100644
>>> --- a/net/ipv6/route.c
>>> +++ b/net/ipv6/route.c
>>> @@ -4590,11 +4590,12 @@ static int fib6_remove_prefsrc(struct fib6_info *rt, void *arg)
>>>  	struct net_device *dev = ((struct arg_dev_net_ip *)arg)->dev;
>>>  	struct net *net = ((struct arg_dev_net_ip *)arg)->net;
>>>  	struct in6_addr *addr = ((struct arg_dev_net_ip *)arg)->addr;
>>> +	u32 tb6_id = l3mdev_fib_table(dev) ? : RT_TABLE_MAIN;
>>>  
>>> -	if (!rt->nh &&
>>> -	    ((void *)rt->fib6_nh->fib_nh_dev == dev || !dev) &&
>>> -	    rt != net->ipv6.fib6_null_entry &&
>>> -	    ipv6_addr_equal(addr, &rt->fib6_prefsrc.addr)) {
>>> +	if (rt != net->ipv6.fib6_null_entry &&
>>> +	    rt->fib6_table->tb6_id == tb6_id &&
>>> +	    ipv6_addr_equal(addr, &rt->fib6_prefsrc.addr) &&
>>> +	    !ipv6_chk_addr(net, addr, rt->fib6_nh->fib_nh_dev, 0)) {
>>>  		spin_lock_bh(&rt6_exception_lock);
>>>  		/* remove prefsrc entry */
>>>  		rt->fib6_prefsrc.plen = 0;
>>
>> The table check is incorrect which is what I was trying to explain here
>> [1]. The route insertion code does not check that the preferred source
>> is accessible from the VRF where the route is installed, but instead
>> that it is accessible from the VRF of the first nexthop device. I'm not
> 
> Sorry for my bad understanding and thanks a lot for your patient response!
> 
> Now I finally get what you mean of "In IPv6, the preferred source address is
> looked up in the same VRF as the first nexthop device." Which is not same with
> the IPv4 commit f96a3d74554d ipv4: Fix incorrect route flushing when source
> address is deleted
> 
> I will remove the tb id checking in next version. Another thing to confirm.
> We need remove the "!rt->nh" checking, right. Because I saw you kept it in you
> reply.
> 

Make sure Ido's test cases for the various cases are added to the test
scripts. Lot of permutations here and we do not want to regress