netdev - Re: [PATCH RESEND v2 net-next 02/14] ipv6: Get rid of RTNL for SIOCDELRT and RTM

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: <3e28015e-0ca0-4933-80b5-de45e3c43b11@redhat.com>
Date: Wed, 16 Apr 2025 10:49:53 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Kuniyuki Iwashima <kuniyu@...zon.com>,
 "David S. Miller" <davem@...emloft.net>, David Ahern <dsahern@...nel.org>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>
Cc: Simon Horman <horms@...nel.org>, Kuniyuki Iwashima <kuni1840@...il.com>,
 netdev@...r.kernel.org
Subject: Re: [PATCH RESEND v2 net-next 02/14] ipv6: Get rid of RTNL for
 SIOCDELRT and RTM_DELROUTE.

On 4/14/25 8:14 PM, Kuniyuki Iwashima wrote:
> Basically, removing an IPv6 route does not require RTNL because
> the IPv6 routing tables are protected by per table lock.
> 
> inet6_rtm_delroute() calls nexthop_find_by_id() to check if the
> nexthop specified by RTA_NH_ID exists.  nexthop uses rbtree and
> the top-down walk can be safely performed under RCU.
> 
> ip6_route_del() already relies on RCU and the table lock, but we
> need to extend the RCU critical section a bit more to cover
> __ip6_del_rt().  For example, nexthop_for_each_fib6_nh() and
> inet6_rt_notify() needs RCU.

The last statement is not clear to me. I don't see __ip6_del_rt()
calling nexthop_for_each_fib6_nh() or inet6_rt_notify() ?!?

Also after this patch we have this chunk in ip6_route_del():

	table = fib6_get_table(cfg->fc_nlinfo.nl_net, cfg->fc_table);
	if (!table)
		//..

	rcu_read_lock();

which AFAICS should be safe because 'table' is freed only at netns exit
time, but acquiring the rcu lock after grabbing the rcu protected struct
is confusing. It should be good adding a comment or moving the rcu lock
before the lookup (and dropping the RCU lock from fib6_get_table())

Thanks,

Paolo