Module Name:    src
Committed By:   ozaki-r
Date:           Sat Feb 24 07:53:15 UTC 2018

Modified Files:
        src/sys/netinet: if_arp.c
        src/sys/netinet6: nd6_nbr.c

Log Message:
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.


To generate a diff of this commit:
cvs rdiff -u -r1.266 -r1.267 src/sys/netinet/if_arp.c
cvs rdiff -u -r1.147 -r1.148 src/sys/netinet6/nd6_nbr.c

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/netinet/if_arp.c
diff -u src/sys/netinet/if_arp.c:1.266 src/sys/netinet/if_arp.c:1.267
--- src/sys/netinet/if_arp.c:1.266	Wed Feb 14 14:15:53 2018
+++ src/sys/netinet/if_arp.c	Sat Feb 24 07:53:15 2018
@@ -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 @@ arp_dad_timer(struct dadq *dp)
 
 	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;
 	}
 

Index: src/sys/netinet6/nd6_nbr.c
diff -u src/sys/netinet6/nd6_nbr.c:1.147 src/sys/netinet6/nd6_nbr.c:1.148
--- src/sys/netinet6/nd6_nbr.c:1.147	Sat Feb 24 07:40:40 2018
+++ src/sys/netinet6/nd6_nbr.c	Sat Feb 24 07:53:15 2018
@@ -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 @@ nd6_dad_timer(struct dadq *dp)
 
 	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;
 	}
 

Reply via email to