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

[Ido Schimmel <idosch@nvidia.com>]

On Fri, Oct 09, 2026 at 02:40:40PM +0900, Daehyeon Ko wrote:
> ipv6_add_addr() checks idev state before allocating an ifaddr, but
> publishes the object later. addrconf_ifdown() can mark and detach the
> idev between the check and publication.
> 
> This happens when a non-loopback device MTU falls below IPV6_MIN_MTU. A
> forced interleaving published an address on a dead idev, and later device
> deletion waited indefinitely for the leaked references.
> 
> Protect the dead indication with idev->lock and keep that lock across
> both hash and device-list publication. Use READ_ONCE() for the initial
> lockless state checks, then recheck both dead and disable_ipv6 before
> publishing. If teardown wins, reject the unpublished object.
> 
> Fixes: 8814c4b53381 ("[IPV6] ADDRCONF: Convert addrconf_lock to RCU.")
> Cc: stable@vger.kernel.org
> Suggested-by: Ido Schimmel <idosch@nvidia.com>
> Assisted-by: LLM
> Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
> ---
>  net/ipv6/addrconf.c | 18 +++++++++++++-----
>  1 file changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index c90ee6dd7446cd..77b3b1154d591c 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -1095,13 +1095,13 @@ ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
>  		return ERR_PTR(-EADDRNOTAVAIL);
>  	}
>  
> -	if (idev->dead) {
> +	if (READ_ONCE(idev->dead)) {
>  		NL_SET_ERR_MSG_MOD(extack, "device is going away");
>  		err = -ENODEV;
>  		goto out;
>  	}
>  
> -	if (idev->cnf.disable_ipv6) {
> +	if (READ_ONCE(idev->cnf.disable_ipv6)) {
>  		NL_SET_ERR_MSG_MOD(extack, "IPv6 is disabled on this device");
>  		err = -EACCES;
>  		goto out;

This hunk is not needed for this fix.

> @@ -1168,14 +1168,20 @@ ipv6_add_addr(struct inet6_dev *idev, struct ifa6_config *cfg,
>  
>  	rcu_read_lock();
>  
> -	err = ipv6_add_addr_hash(idev->dev, ifa);
> +	write_lock_bh(&idev->lock);
> +
> +	if (idev->dead)
> +		err = -ENODEV;
> +	else if (READ_ONCE(idev->cnf.disable_ipv6))
> +		err = -EACCES;

I asked why this check is needed and you didn't respond:

https://lore.kernel.org/netdev/20261007164609.GB1153540@shredder/

And I provided a suggested diff on v1 that doesn't check 'disable_ipv6':

https://lore.kernel.org/all/20261004135117.GA206930@shredder/

If you disagree with the suggestions, then say so instead of posting a
new version that ignores what was suggested.

> +	else
> +		err = ipv6_add_addr_hash(idev->dev, ifa);
>  	if (err < 0) {
> +		write_unlock_bh(&idev->lock);
>  		rcu_read_unlock();
>  		goto out;
>  	}
>  
> -	write_lock_bh(&idev->lock);
> -
>  	/* Add to inet6_dev unicast addr list. */
>  	ipv6_link_dev_addr(idev, ifa);
>  
> @@ -3897,7 +3903,9 @@ static int addrconf_ifdown(struct net_device *dev, bool unregister)
>  	 *	   Do not dev_put!
>  	 */
>  	if (unregister) {
> +		write_lock_bh(&idev->lock);
>  		WRITE_ONCE(idev->dead, 1);
> +		write_unlock_bh(&idev->lock);
>  
>  		/* protected by rtnl_lock */
>  		RCU_INIT_POINTER(dev->ip6_ptr, NULL);
> -- 
> 2.55.0
>