[src/trunk]: src/sys Avoid a race condition of DAD timer destructions

[ozaki-r <ozaki-r%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/971523079250
branches:  trunk
changeset: 830050:971523079250
user:      ozaki-r <ozaki-r%NetBSD.org@localhost>
date:      Sat Feb 24 07:53:15 2018 +0000

description:
Avoid a race condition of DAD timer destructions

When we see dp->dad_ifa == NULL, it means that the ifa is being deleted and also
the callout is scheduled again by someone.  We shouldn't rely on a result of
callout_pending to know if the callout is scheduled because it returns false if
the subsequent callout handler is already on the fly.

We have to always delegate the destruction of dp to the subsequent handler
unconditionally if dp->dad_ifa == NULL. Otherwise, the first handler destroys
the dp and the second handler tries to handle destroyed dp.

diffstat:

 sys/netinet/if_arp.c   |  17 ++++++-----------
 sys/netinet6/nd6_nbr.c |  17 ++++++-----------
 2 files changed, 12 insertions(+), 22 deletions(-)

diffs (76 lines):

diff -r 19f86cc79c33 -r 971523079250 sys/netinet/if_arp.c
--- a/sys/netinet/if_arp.c      Sat Feb 24 07:40:40 2018 +0000
+++ b/sys/netinet/if_arp.c      Sat Feb 24 07:53:15 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: if_arp.c,v 1.266 2018/02/14 14:15:53 maxv Exp $        */
+/*     $NetBSD: if_arp.c,v 1.267 2018/02/24 07:53:15 ozaki-r Exp $     */
 
 /*
  * Copyright (c) 1998, 2000, 2008 The NetBSD Foundation, Inc.
@@ -68,7 +68,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.266 2018/02/14 14:15:53 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.267 2018/02/24 07:53:15 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_ddb.h"
@@ -1673,15 +1673,10 @@
 
        ifa = dp->dad_ifa;
        if (ifa == NULL) {
-               /* ifa is being deleted. DAD should be freed too. */
-               if (callout_pending(&dp->dad_timer_ch)) {
-                       /*
-                        * The callout is scheduled again, so we cannot destroy
-                        * dp in this run.
-                        */
-                       goto done;
-               }
-               need_free = true;
+               /*
+                * ifa is being deleted and the callout is already scheduled
+                * again, so we cannot destroy dp in this run.
+                */
                goto done;
        }
 
diff -r 19f86cc79c33 -r 971523079250 sys/netinet6/nd6_nbr.c
--- a/sys/netinet6/nd6_nbr.c    Sat Feb 24 07:40:40 2018 +0000
+++ b/sys/netinet6/nd6_nbr.c    Sat Feb 24 07:53:15 2018 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: nd6_nbr.c,v 1.147 2018/02/24 07:40:40 ozaki-r Exp $    */
+/*     $NetBSD: nd6_nbr.c,v 1.148 2018/02/24 07:53:15 ozaki-r Exp $    */
 /*     $KAME: nd6_nbr.c,v 1.61 2001/02/10 16:06:14 jinmei Exp $        */
 
 /*
@@ -31,7 +31,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: nd6_nbr.c,v 1.147 2018/02/24 07:40:40 ozaki-r Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nd6_nbr.c,v 1.148 2018/02/24 07:53:15 ozaki-r Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_inet.h"
@@ -1231,15 +1231,10 @@
 
        ifa = dp->dad_ifa;
        if (ifa == NULL) {
-               /* ifa is being deleted. DAD should be freed too. */
-               if (callout_pending(&dp->dad_timer_ch)) {
-                       /*
-                        * The callout is scheduled again, so we cannot destroy
-                        * dp in this run.
-                        */
-                       goto done;
-               }
-               need_free = true;
+               /*
+                * ifa is being deleted and the callout is already scheduled
+                * again, so we cannot destroy dp in this run.
+                */
                goto done;
        }