net/ipv6/addrconf.c | 6 ++++++ 1 file changed, 6 insertions(+)
addrconf_dad_end() transitions ifp->state from DAD to POSTDAD under
ifp->lock and releases the lock. addrconf_dad_failure() takes
ifp->lock again with the spin_lock_bh() following the
net_info_ratelimited() duplicate-address log. A concurrent
ipv6_del_addr() can acquire 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
Bail out at errdad: when ifp->state is already DEAD. The existing
in6_ifa_put() releases the reference taken for this invocation.
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
net/ipv6/addrconf.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 5476b6536eb7..14b1ab43da87 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2227,6 +2227,12 @@ 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) {
+ /* ipv6_del_addr() already removed ifp while lock was dropped */
+ spin_unlock_bh(&ifp->lock);
+ in6_ifa_put(ifp);
+ return;
+ }
ifp->state = INET6_IFADDR_STATE_ERRDAD;
spin_unlock_bh(&ifp->lock);
--
2.25.1addrconf_dad_end() transitions ifp->state from DAD to POSTDAD under
ifp->lock and releases the lock. addrconf_dad_failure() takes
ifp->lock again with the spin_lock_bh() following the
net_info_ratelimited() duplicate-address log. A concurrent
ipv6_del_addr() can acquire 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
Bail out at errdad: when ifp->state is already DEAD. The existing
in6_ifa_put() releases the reference taken for this invocation.
Fixes: c15b1ccadb32 ("ipv6: move DAD and addrconf_verify processing to workqueue")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
net/ipv6/addrconf.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
index 5476b6536eb7..14b1ab43da87 100644
--- a/net/ipv6/addrconf.c
+++ b/net/ipv6/addrconf.c
@@ -2227,6 +2227,12 @@ 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) {
+ /* ipv6_del_addr() already removed ifp while lock was dropped */
+ spin_unlock_bh(&ifp->lock);
+ in6_ifa_put(ifp);
+ return;
+ }
ifp->state = INET6_IFADDR_STATE_ERRDAD;
spin_unlock_bh(&ifp->lock);
--
2.25.12026-04-21, 15:50:33 +0800, Linmao Li wrote: > addrconf_dad_end() transitions ifp->state from DAD to POSTDAD under > ifp->lock and releases the lock. addrconf_dad_failure() takes > ifp->lock again with the spin_lock_bh() following the > net_info_ratelimited() duplicate-address log. A concurrent > ipv6_del_addr() can acquire the lock in that window, set ifp->state > to DEAD and run list_del_rcu(&ifp->if_list). You're pretty much saying that the ifp->state check we did in addrconf_dad_end before dropping the lock is not valid, so it seems we should just skip that separate check since it's not doing anything useful, and move it under the "main" lock we acquire after the net_info_ratelimited(). There would still be a problem with "we dropped the lock in the STABLE_PRIVACY block", which your patch handles. > 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 > > Bail out at errdad: when ifp->state is already DEAD. The existing > in6_ifa_put() releases the reference taken for this invocation. Mentioning "the existing in6_ifa_put()" is a bit confusing since you're adding a separate unlock/put/return path. -- Sabrina
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 around address regeneration,
so add a state-is-DEAD bail-out right after the lock is re-taken
at lock_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..b58bd9f11606 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;
@@ -2223,6 +2225,11 @@ void addrconf_dad_failure(struct sk_buff *skb, struct inet6_ifaddr *ifp)
in6_ifa_put(ifp2);
lock_errdad:
spin_lock_bh(&ifp->lock);
+ if (ifp->state == INET6_IFADDR_STATE_DEAD) {
+ spin_unlock_bh(&ifp->lock);
+ in6_ifa_put(ifp);
+ return;
+ }
}
errdad:
--
2.25.1On Wed, Apr 29, 2026 at 09:26:51AM +0800, Linmao Li wrote:
> diff --git a/net/ipv6/addrconf.c b/net/ipv6/addrconf.c
> index 5476b6536eb7..b58bd9f11606 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;
> @@ -2223,6 +2225,11 @@ void addrconf_dad_failure(struct sk_buff *skb, struct inet6_ifaddr *ifp)
> in6_ifa_put(ifp2);
> lock_errdad:
> spin_lock_bh(&ifp->lock);
> + if (ifp->state == INET6_IFADDR_STATE_DEAD) {
The code below expects the state to be POSTDAD, so wouldn't it be more
robust to check for anything but POSTDAD rather than specifically
checking for DEAD? Otherwise we risk overwriting a state that was set
while we weren't holding the lock.
Also:
1. Please post new versions in a new thread:
https://docs.kernel.org/process/maintainer-netdev.html#resending-after-review
2. Please include a change log:
https://docs.kernel.org/process/maintainer-netdev.html#changes-requested
> + spin_unlock_bh(&ifp->lock);
> + in6_ifa_put(ifp);
> + return;
> + }
> }
>
> errdad:
> --
> 2.25.1
>
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.1On 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
© 2016 - 2026 Red Hat, Inc.