netdev - Re: [PATCHv2 net] 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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ZLgEF9w903kI5pHs@shredder>
Date: Wed, 19 Jul 2023 18:41:11 +0300
From: Ido Schimmel <idosch@...sch.org>
To: Hangbin Liu <liuhangbin@...il.com>
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: [PATCHv2 net] ipv6: do not match device when remove source route

On Wed, Jul 19, 2023 at 05:54:49PM +0800, Hangbin Liu wrote:
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index 64e873f5895f..4f49677e24a2 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -4590,10 +4590,11 @@ 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;

l3mdev_fib_table() handles the case of 'dev' being NULL, so this looks
OK.

>  
>  	if (!rt->nh &&
> -	    ((void *)rt->fib6_nh->fib_nh_dev == dev || !dev) &&

Now that we are not checking the nexthop device, I believe we should
remove the '!rt->nh' check. With this check, the source address is not
removed from IPv6 routes that are using a nexthop object. This is in
contrast to IPv4 [1]. After removing the check, IPv4 and IPv6 are
consistent (disregarding the fundamental difference of removing the
route vs. only the source address) [2].

>  	    rt != net->ipv6.fib6_null_entry &&
> +	    rt->fib6_table->tb6_id == tb6_id &&
>  	    ipv6_addr_equal(addr, &rt->fib6_prefsrc.addr)) {
>  		spin_lock_bh(&rt6_exception_lock);
>  		/* remove prefsrc entry */

[1]
+ ip link add name dummy1 up type dummy
+ ip link add name dummy2 up type dummy
+ ip -4 nexthop add id 1 dev dummy1
+ ip -6 nexthop add id 2 dev dummy1
+ ip address add 192.0.2.1/24 dev dummy2
+ ip address add 2001:db8:1::1/64 dev dummy2
+ ip route add 198.51.100.0/24 nhid 1 src 192.0.2.1
+ ip route add 2001:db8:10::/64 nhid 2 src 2001:db8:1::1
+ ip -4 route show 198.51.100.0/24
198.51.100.0/24 nhid 1 dev dummy1 src 192.0.2.1 
+ ip -6 route show 2001:db8:10::/64
2001:db8:10::/64 nhid 2 dev dummy1 src 2001:db8:1::1 metric 1024 pref medium
+ ip address del 192.0.2.1/24 dev dummy2
+ ip address del 2001:db8:1::1/64 dev dummy2
+ ip -4 route show 198.51.100.0/24
+ ip -6 route show 2001:db8:10::/64
2001:db8:10::/64 nhid 2 dev dummy1 src 2001:db8:1::1 metric 1024 pref medium

[2]
+ ip link add name dummy1 up type dummy
+ ip link add name dummy2 up type dummy
+ ip -4 nexthop add id 1 dev dummy1
+ ip -6 nexthop add id 2 dev dummy1
+ ip address add 192.0.2.1/24 dev dummy2
+ ip address add 2001:db8:1::1/64 dev dummy2
+ ip route add 198.51.100.0/24 nhid 1 src 192.0.2.1
+ ip route add 2001:db8:10::/64 nhid 2 src 2001:db8:1::1
+ ip -4 route show 198.51.100.0/24
198.51.100.0/24 nhid 1 dev dummy1 src 192.0.2.1 
+ ip -6 route show 2001:db8:10::/64
2001:db8:10::/64 nhid 2 dev dummy1 src 2001:db8:1::1 metric 1024 pref medium
+ ip address del 192.0.2.1/24 dev dummy2
+ ip address del 2001:db8:1::1/64 dev dummy2
+ ip -4 route show 198.51.100.0/24
+ ip -6 route show 2001:db8:10::/64
2001:db8:10::/64 nhid 2 dev dummy1 metric 1024 pref medium