| From 63780990b77e3fb5d915ba7c16e823f1ce088a72 Mon Sep 17 00:00:00 2001 |
| From: Sasha Levin <sashal@kernel.org> |
| Date: Wed, 2 Jul 2025 16:01:20 -0700 |
| Subject: ipv6: mcast: Check inet6_dev->dead under idev->mc_lock in |
| __ipv6_dev_mc_inc(). |
| |
| From: Kuniyuki Iwashima <kuniyu@google.com> |
| |
| [ Upstream commit dbd40f318cf2f59759bd170c401adc20ba360a3e ] |
| |
| Since commit 63ed8de4be81 ("mld: add mc_lock for protecting |
| per-interface mld data"), every multicast resource is protected |
| by inet6_dev->mc_lock. |
| |
| RTNL is unnecessary in terms of protection but still needed for |
| synchronisation between addrconf_ifdown() and __ipv6_dev_mc_inc(). |
| |
| Once we removed RTNL, there would be a race below, where we could |
| add a multicast address to a dead inet6_dev. |
| |
| CPU1 CPU2 |
| ==== ==== |
| addrconf_ifdown() __ipv6_dev_mc_inc() |
| if (idev->dead) <-- false |
| dead = true return -ENODEV; |
| ipv6_mc_destroy_dev() / ipv6_mc_down() |
| mutex_lock(&idev->mc_lock) |
| ... |
| mutex_unlock(&idev->mc_lock) |
| mutex_lock(&idev->mc_lock) |
| ... |
| mutex_unlock(&idev->mc_lock) |
| |
| The race window can be easily closed by checking inet6_dev->dead |
| under inet6_dev->mc_lock in __ipv6_dev_mc_inc() as addrconf_ifdown() |
| will acquire it after marking inet6_dev dead. |
| |
| Let's check inet6_dev->dead under mc_lock in __ipv6_dev_mc_inc(). |
| |
| Note that now __ipv6_dev_mc_inc() no longer depends on RTNL and |
| we can remove ASSERT_RTNL() there and the RTNL comment above |
| addrconf_join_solict(). |
| |
| Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com> |
| Reviewed-by: Eric Dumazet <edumazet@google.com> |
| Link: https://patch.msgid.link/20250702230210.3115355-4-kuni1840@gmail.com |
| Signed-off-by: Jakub Kicinski <kuba@kernel.org> |
| Signed-off-by: Sasha Levin <sashal@kernel.org> |
| --- |
| net/ipv6/addrconf.c | 7 +++---- |
| net/ipv6/mcast.c | 11 +++++------ |
| 2 files changed, 8 insertions(+), 10 deletions(-) |
| |
| diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c |
| index 69915bb8b96d..cbdb510b40ea 100644 |
| --- a/net/ipv6/addrconf.c |
| +++ b/net/ipv6/addrconf.c |
| @@ -2185,13 +2185,12 @@ void addrconf_dad_failure(struct sk_buff *skb, struct inet6_ifaddr *ifp) |
| in6_ifa_put(ifp); |
| } |
| |
| -/* Join to solicited addr multicast group. |
| - * caller must hold RTNL */ |
| +/* Join to solicited addr multicast group. */ |
| void addrconf_join_solict(struct net_device *dev, const struct in6_addr *addr) |
| { |
| struct in6_addr maddr; |
| |
| - if (dev->flags&(IFF_LOOPBACK|IFF_NOARP)) |
| + if (READ_ONCE(dev->flags) & (IFF_LOOPBACK | IFF_NOARP)) |
| return; |
| |
| addrconf_addr_solict_mult(addr, &maddr); |
| @@ -3807,7 +3806,7 @@ static int addrconf_ifdown(struct net_device *dev, bool unregister) |
| * Do not dev_put! |
| */ |
| if (unregister) { |
| - idev->dead = 1; |
| + WRITE_ONCE(idev->dead, 1); |
| |
| /* protected by rtnl_lock */ |
| RCU_INIT_POINTER(dev->ip6_ptr, NULL); |
| diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c |
| index e9e59a83ba9b..e7f569875e71 100644 |
| --- a/net/ipv6/mcast.c |
| +++ b/net/ipv6/mcast.c |
| @@ -906,23 +906,22 @@ static struct ifmcaddr6 *mca_alloc(struct inet6_dev *idev, |
| static int __ipv6_dev_mc_inc(struct net_device *dev, |
| const struct in6_addr *addr, unsigned int mode) |
| { |
| - struct ifmcaddr6 *mc; |
| struct inet6_dev *idev; |
| - |
| - ASSERT_RTNL(); |
| + struct ifmcaddr6 *mc; |
| |
| /* we need to take a reference on idev */ |
| idev = in6_dev_get(dev); |
| - |
| if (!idev) |
| return -EINVAL; |
| |
| - if (idev->dead) { |
| + mutex_lock(&idev->mc_lock); |
| + |
| + if (READ_ONCE(idev->dead)) { |
| + mutex_unlock(&idev->mc_lock); |
| in6_dev_put(idev); |
| return -ENODEV; |
| } |
| |
| - mutex_lock(&idev->mc_lock); |
| for_each_mc_mclock(idev, mc) { |
| if (ipv6_addr_equal(&mc->mca_addr, addr)) { |
| mc->mca_users++; |
| -- |
| 2.39.5 |
| |