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]

Date: Mon, 14 Aug 2023 16:33:37 +0800
From: Hangbin Liu <liuhangbin@...il.com>
To: Ido Schimmel <idosch@...sch.org>
Cc: netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>,
	David Ahern <dsahern@...nel.org>,
	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

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.

Thanks and Best regards
Hangbin