Re: [PATCH net v3 1/3] ipv6: serialize address publication with device teardown

On Sat, Oct 10, 2026 at 05:43:14AM +0000, netdev-bot+sashiko@kernel.org wrote:

[Severity: High]
This is a pre-existing issue and was not introduced by this patch. The
recheck above does not cover the window after ipv6_add_addr() returns to
addrconf_dad_failure().

addrconf_dad_failure() runs from ndisc_recv_ns()/ndisc_recv_na() in
softirq without RTNL. For IFA_F_STABLE_PRIVACY addresses it does:

net/ipv6/addrconf.c:addrconf_dad_failure() {
    ...
		ifp2 = ipv6_add_addr(idev, &cfg, false, NULL);
		if (IS_ERR(ifp2))
			goto lock_errdad;

		spin_lock_bh(&ifp2->lock);
		ifp2->stable_privacy_retry = retries;
		ifp2->state = INET6_IFADDR_STATE_PREDAD;
		spin_unlock_bh(&ifp2->lock);

		addrconf_mod_dad_work(ifp2, net->ipv6.sysctl.idgen_delay);
    ...
}

addrconf_ifdown() can run on another CPU because of NETDEV_DOWN,
disable_ipv6, or NETDEV_CHANGEMTU below IPV6_MIN_MTU. If it snapshots
addr_list after ifp2 is linked, it can tear ifp2 down completely before
the spin_lock_bh(&ifp2->lock) above:

CPU1 (addrconf_ifdown)
    ifa->state = INET6_IFADDR_STATE_DEAD;
    __ipv6_ifa_notify(RTM_DELADDR, ifa)
        ip6_del_rt(net, ifp->rt, false);
        ifp->rt = NULL;
    list_del_rcu(&ifa->if_list);
    in6_ifa_put(ifa);

CPU2 (addrconf_dad_failure)
    ifp2->state = INET6_IFADDR_STATE_PREDAD;
    addrconf_mod_dad_work(ifp2, ...);

Can CPU2 overwrite the DEAD state here and queue DAD work on an address
that has already been torn down?

Suppose IF_READY is still set and ifp2 has inherited IFA_F_OPTIMISTIC.
Then addrconf_dad_work()->addrconf_dad_begin() reaches:

	if (ifp->flags & IFA_F_OPTIMISTIC) {
		ip6_ins_rt(net, ifp->rt);

At this point ifp->rt is NULL, and __ip6_ins_rt() dereferences
rt->fib6_table. IF_READY stays set in two cases:

- the NETDEV_CHANGEMTU path calls addrconf_ifdown(dev, true) without a
  prior NETDEV_DOWN
- the link comes back up within idgen_delay

Otherwise the address is left in state DAD or POSTDAD. On the final put,
inet6_ifa_finish_destroy() then hits:

	if (ifp->state != INET6_IFADDR_STATE_DEAD) {
		pr_warn("Freeing alive inet6 address %p\n", ifp);
		return;
	}

so the ifaddr is never freed.

addrconf_dad_start() avoids this by checking for DEAD first:

	if (ifp->state != INET6_IFADDR_STATE_DEAD) {
		ifp->state = INET6_IFADDR_STATE_PREDAD;
		begin_dad = true;
	}

Would a similar check under ifp2->lock in addrconf_dad_failure() work?
The PREDAD write itself looks redundant, because ipv6_add_addr() already
returns the object in state 0 (PREDAD). No later patch in this series
changes addrconf_dad_failure().

Looks like a valid issue. Please fix in patch #4 in v4 with something
like this:

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 7bd0e8820abb..7cb156ddc191 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c

@@ -2230,7 +2230,6 @@ void addrconf_dad_failure(struct sk_buff *skb, struct inet6_ifaddr *ifp)
 
 		spin_lock_bh(&ifp2->lock);
 		ifp2->stable_privacy_retry = retries;
-		ifp2->state = INET6_IFADDR_STATE_PREDAD;
 		spin_unlock_bh(&ifp2->lock);
 
 		addrconf_mod_dad_work(ifp2, net->ipv6.sysctl.idgen_delay);