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

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] [day] [month] [year] [list]

Message-ID: <20250619185645.1647494-1-kuni1840@gmail.com>
Date: Thu, 19 Jun 2025 11:56:08 -0700
From: Kuniyuki Iwashima <kuni1840@...il.com>
To: pabeni@...hat.com
Cc: davem@...emloft.net,
	dsahern@...nel.org,
	edumazet@...gle.com,
	horms@...nel.org,
	kuba@...nel.org,
	kuni1840@...il.com,
	kuniyu@...gle.com,
	netdev@...r.kernel.org
Subject: 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@...hat.com>
Date: Thu, 19 Jun 2025 14:33:56 +0200
> On 6/17/25 1:28 AM, Kuniyuki Iwashima wrote:
> > From: Kuniyuki Iwashima <kuniyu@...gle.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);
 	}