[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/8137b86ba0c5
branches:  trunk
changeset: 359749:8137b86ba0c5
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 921e109ecfba -r 8137b86ba0c5 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 921e109ecfba -r 8137b86ba0c5 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;
        }