Message ID | 8ab821e36073a4a406c50ec83c9e8dc586c539e4.1712585809.git.jbenc@redhat.com (mailing list archive) |
---|---|
State | Accepted |
Commit | 7633c4da919ad51164acbf1aa322cc1a3ead6129 |
Delegated to: | Netdev Maintainers |
Headers | show |
Series | [net,v2] ipv6: fix race condition between ipv6_get_ifaddr and ipv6_del_addr | expand |
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Mon, 8 Apr 2024 16:18:21 +0200 you wrote: > Although ipv6_get_ifaddr walks inet6_addr_lst under the RCU lock, it > still means hlist_for_each_entry_rcu can return an item that got removed > from the list. The memory itself of such item is not freed thanks to RCU > but nothing guarantees the actual content of the memory is sane. > > In particular, the reference count can be zero. This can happen if > ipv6_del_addr is called in parallel. ipv6_del_addr removes the entry > from inet6_addr_lst (hlist_del_init_rcu(&ifp->addr_lst)) and drops all > references (__in6_ifa_put(ifp) + in6_ifa_put(ifp)). With bad enough > timing, this can happen: > > [...] Here is the summary with links: - [net,v2] ipv6: fix race condition between ipv6_get_ifaddr and ipv6_del_addr https://git.kernel.org/netdev/net/c/7633c4da919a You are awesome, thank you!
diff --git a/include/net/addrconf.h b/include/net/addrconf.h index 9d06eb945509..62a407db1bf5 100644 --- a/include/net/addrconf.h +++ b/include/net/addrconf.h @@ -438,6 +438,10 @@ static inline void in6_ifa_hold(struct inet6_ifaddr *ifp) refcount_inc(&ifp->refcnt); } +static inline bool in6_ifa_hold_safe(struct inet6_ifaddr *ifp) +{ + return refcount_inc_not_zero(&ifp->refcnt); +} /* * compute link-local solicited-node multicast address diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c index 92db9b474f2b..779aa6ecdd49 100644 --- a/net/ipv6/addrconf.c +++ b/net/ipv6/addrconf.c @@ -2091,9 +2091,10 @@ struct inet6_ifaddr *ipv6_get_ifaddr(struct net *net, const struct in6_addr *add if (ipv6_addr_equal(&ifp->addr, addr)) { if (!dev || ifp->idev->dev == dev || !(ifp->scope&(IFA_LINK|IFA_HOST) || strict)) { - result = ifp; - in6_ifa_hold(ifp); - break; + if (in6_ifa_hold_safe(ifp)) { + result = ifp; + break; + } } } }