1. Patchew
  2. linux
  3. View series

[PATCH net v3] ipv6: addrconf: skip ERRDAD transition when address already DEAD

Linmao Li posted 1 patch 1 month, 3 weeks ago
There is a newer version of this series
net/ipv6/addrconf.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
[PATCH net v3] ipv6: addrconf: skip ERRDAD transition when address already DEAD
Posted by Linmao Li 1 month, 3 weeks ago 
addrconf_dad_failure() transitions ifp->state from DAD to POSTDAD
via addrconf_dad_end(), which drops ifp->lock on return.  The lock
is re-acquired after net_info_ratelimited().  A concurrent
ipv6_del_addr() can take the lock in that window, set ifp->state
to DEAD and run list_del_rcu(&ifp->if_list).

addrconf_dad_failure() then overwrites DEAD with ERRDAD at errdad:
and schedules a new dad_work.  The work calls ipv6_del_addr()
again, hitting the already-poisoned list entry:

  general protection fault: 0000 [#1] SMP NOPTI
  CPU: 4 PID: 217 Comm: kworker/4:1
  Workqueue: ipv6_addrconf addrconf_dad_work
  RIP: 0010:ipv6_del_addr+0xe9/0x280
  RAX: dead000000000122
  Call Trace:
   addrconf_dad_stop+0x113/0x140
   addrconf_dad_work+0x28c/0x430
   process_one_work+0x1eb/0x3b0
   worker_thread+0x4d/0x400
   kthread+0x104/0x140
   ret_from_fork+0x35/0x40

Fold the addrconf_dad_end() logic into addrconf_dad_failure()
under a single ifp->lock critical section.  The STABLE_PRIVACY
branch temporarily drops ifp->lock, so keep a state-is-DEAD
bail-out at errdad: for that remaining window.

Fixes: c15b1ccadb32 ("ipv6: move DAD and addrconf_verify processing to workqueue")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
 net/ipv6/addrconf.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 5476b6536eb7..c9ea0d5042d0 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2166,16 +2166,18 @@ void addrconf_dad_failure(struct sk_buff *skb, struct inet6_ifaddr *ifp)
 	struct net *net = dev_net(idev->dev);
 	int max_addresses;
 
-	if (addrconf_dad_end(ifp)) {
+	spin_lock_bh(&ifp->lock);
+
+	if (ifp->state != INET6_IFADDR_STATE_DAD) {
+		spin_unlock_bh(&ifp->lock);
 		in6_ifa_put(ifp);
 		return;
 	}
+	ifp->state = INET6_IFADDR_STATE_POSTDAD;
 
 	net_info_ratelimited("%s: IPv6 duplicate address %pI6c used by %pM detected!\n",
 			     ifp->idev->dev->name, &ifp->addr, eth_hdr(skb)->h_source);
 
-	spin_lock_bh(&ifp->lock);
-
 	if (ifp->flags & IFA_F_STABLE_PRIVACY) {
 		struct in6_addr new_addr;
 		struct inet6_ifaddr *ifp2;
@@ -2227,6 +2229,11 @@ void addrconf_dad_failure(struct sk_buff *skb, struct inet6_ifaddr *ifp)
 
 errdad:
 	/* transition from _POSTDAD to _ERRDAD */
+	if (ifp->state == INET6_IFADDR_STATE_DEAD) {
+		spin_unlock_bh(&ifp->lock);
+		in6_ifa_put(ifp);
+		return;
+	}
 	ifp->state = INET6_IFADDR_STATE_ERRDAD;
 	spin_unlock_bh(&ifp->lock);
 
-- 
2.25.1
Re: [PATCH net v3] ipv6: addrconf: skip ERRDAD transition when address already DEAD
Posted by Paolo Abeni 1 month, 2 weeks ago 
On 4/23/26 4:32 AM, Linmao Li wrote:
> addrconf_dad_failure() transitions ifp->state from DAD to POSTDAD
> via addrconf_dad_end(), which drops ifp->lock on return.  The lock
> is re-acquired after net_info_ratelimited().  A concurrent
> ipv6_del_addr() can take the lock in that window, set ifp->state
> to DEAD and run list_del_rcu(&ifp->if_list).
> 
> addrconf_dad_failure() then overwrites DEAD with ERRDAD at errdad:
> and schedules a new dad_work.  The work calls ipv6_del_addr()
> again, hitting the already-poisoned list entry:
> 
>   general protection fault: 0000 [#1] SMP NOPTI
>   CPU: 4 PID: 217 Comm: kworker/4:1
>   Workqueue: ipv6_addrconf addrconf_dad_work
>   RIP: 0010:ipv6_del_addr+0xe9/0x280
>   RAX: dead000000000122
>   Call Trace:
>    addrconf_dad_stop+0x113/0x140
>    addrconf_dad_work+0x28c/0x430
>    process_one_work+0x1eb/0x3b0
>    worker_thread+0x4d/0x400
>    kthread+0x104/0x140
>    ret_from_fork+0x35/0x40
> 
> Fold the addrconf_dad_end() logic into addrconf_dad_failure()
> under a single ifp->lock critical section.  The STABLE_PRIVACY
> branch temporarily drops ifp->lock, so keep a state-is-DEAD
> bail-out at errdad: for that remaining window.
> 
> Fixes: c15b1ccadb32 ("ipv6: move DAD and addrconf_verify processing to workqueue")
> Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
> ---
>  net/ipv6/addrconf.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 5476b6536eb7..c9ea0d5042d0 100644
> --- a/net/ipv6/addrconf.c
> +++ b/net/ipv6/addrconf.c
> @@ -2166,16 +2166,18 @@ void addrconf_dad_failure(struct sk_buff *skb, struct inet6_ifaddr *ifp)
>  	struct net *net = dev_net(idev->dev);
>  	int max_addresses;
>  
> -	if (addrconf_dad_end(ifp)) {
> +	spin_lock_bh(&ifp->lock);
> +
> +	if (ifp->state != INET6_IFADDR_STATE_DAD) {
> +		spin_unlock_bh(&ifp->lock);
>  		in6_ifa_put(ifp);
>  		return;
>  	}
> +	ifp->state = INET6_IFADDR_STATE_POSTDAD;
>  
>  	net_info_ratelimited("%s: IPv6 duplicate address %pI6c used by %pM detected!\n",
>  			     ifp->idev->dev->name, &ifp->addr, eth_hdr(skb)->h_source);
>  
> -	spin_lock_bh(&ifp->lock);
> -
>  	if (ifp->flags & IFA_F_STABLE_PRIVACY) {
>  		struct in6_addr new_addr;
>  		struct inet6_ifaddr *ifp2;
> @@ -2227,6 +2229,11 @@ void addrconf_dad_failure(struct sk_buff *skb, struct inet6_ifaddr *ifp)
>  
>  errdad:
>  	/* transition from _POSTDAD to _ERRDAD */
> +	if (ifp->state == INET6_IFADDR_STATE_DEAD) {
> +		spin_unlock_bh(&ifp->lock);
> +		in6_ifa_put(ifp);
> +		return;

It looks like this check is need only when the ifp->lock is released
again, i.e. just after the `lock_errdad`. Please move it there, to avoid
confusion when looking at this code in the future.

Thanks,

Paolo

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.