Re: [PATCH v1 net-next 06/15] ipv6: mcast: Don't hold RTNL for IPV6_ADD_MEMBERSHIP and MCAST_JOIN_GROUP.

From: Paolo Abeni <pabeni@redhat.com>
Date: Thu, 19 Jun 2025 14:33:56 +0200

On 6/17/25 1:28 AM, Kuniyuki Iwashima wrote:

quoted

From: Kuniyuki Iwashima <kuniyu@google.com>

In __ipv6_sock_mc_join(), per-socket mld data is protected by lock_sock(),
and only __dev_get_by_index() requires RTNL.

Let's use dev_get_by_index() and drop RTNL for IPV6_ADD_MEMBERSHIP and
MCAST_JOIN_GROUP.

Note that when we fetch dev from rt6_lookup(), we can call dev_hold()
safely for rt->dst.dev as it already holds refcnt for the dev if exists.

I'm not 110% sure it's safe. AFAICS a racing dst_dev_put() could
potentially release the rt->dst reference concurrently???
The race looks quite orthogonal to the patchset, but double-checking I'm
not missing anything.

Oh, I missed that one.  Given some ->err_handler() use rt6_lookup()
under RCU, adding rcu_read_lock() could be safer ?

diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
index c91134c7e927..fc49f84d74ca 100644
--- a/net/ipv6/mcast.c
+++ b/net/ipv6/mcast.c

@@ -201,12 +201,14 @@ static int __ipv6_sock_mc_join(struct sock *sk, int ifindex,
 	if (ifindex == 0) {
 		struct rt6_info *rt;
 
+		rcu_read_lock();
 		rt = rt6_lookup(net, addr, NULL, 0, NULL, 0);
 		if (rt) {
 			dev = rt->dst.dev;
 			dev_hold(dev);
 			ip6_rt_put(rt);
 		}
+		rcu_read_unlock();
 	} else {
 		dev = dev_get_by_index(net, ifindex);
 	}