Module Name: src
Committed By: martin
Date: Mon Feb 26 13:36:01 UTC 2018
Modified Files:
src/sys/netinet [netbsd-8]: if_arp.c
src/sys/netinet6 [netbsd-8]: nd6_nbr.c
Log Message:
Pull up following revision(s) (requested by ozaki-r in ticket #589):
sys/netinet/if_arp.c: revision 1.267
sys/netinet6/nd6_nbr.c: revision 1.146-1.148
Use KASSERT for checking a programming error
Simplify; pass dp to nd6_dad_duplicated instead of looking it up again in it
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.250.2.5 -r1.250.2.6 src/sys/netinet/if_arp.c
cvs rdiff -u -r1.138.6.3 -r1.138.6.4 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.250.2.5 src/sys/netinet/if_arp.c:1.250.2.6
--- src/sys/netinet/if_arp.c:1.250.2.5 Fri Jan 26 15:41:12 2018
+++ src/sys/netinet/if_arp.c Mon Feb 26 13:36:01 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: if_arp.c,v 1.250.2.5 2018/01/26 15:41:12 martin Exp $ */
+/* $NetBSD: if_arp.c,v 1.250.2.6 2018/02/26 13:36:01 martin 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.250.2.5 2018/01/26 15:41:12 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.250.2.6 2018/02/26 13:36:01 martin Exp $");
#ifdef _KERNEL_OPT
#include "opt_ddb.h"
@@ -1686,15 +1686,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.138.6.3 src/sys/netinet6/nd6_nbr.c:1.138.6.4
--- src/sys/netinet6/nd6_nbr.c:1.138.6.3 Fri Feb 2 12:55:08 2018
+++ src/sys/netinet6/nd6_nbr.c Mon Feb 26 13:36:01 2018
@@ -1,4 +1,4 @@
-/* $NetBSD: nd6_nbr.c,v 1.138.6.3 2018/02/02 12:55:08 martin Exp $ */
+/* $NetBSD: nd6_nbr.c,v 1.138.6.4 2018/02/26 13:36:01 martin 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.138.6.3 2018/02/02 12:55:08 martin Exp $");
+__KERNEL_RCSID(0, "$NetBSD: nd6_nbr.c,v 1.138.6.4 2018/02/26 13:36:01 martin Exp $");
#ifdef _KERNEL_OPT
#include "opt_inet.h"
@@ -84,7 +84,7 @@ static void nd6_dad_timer(struct dadq *)
static void nd6_dad_ns_output(struct dadq *, struct ifaddr *);
static void nd6_dad_ns_input(struct ifaddr *);
static void nd6_dad_na_input(struct ifaddr *);
-static void nd6_dad_duplicated(struct ifaddr *);
+static void nd6_dad_duplicated(struct dadq *);
static int dad_ignore_ns = 0; /* ignore NS in DAD - specwise incorrect*/
static int dad_maxtry = 15; /* max # of *tries* to transmit DAD packet */
@@ -1092,10 +1092,13 @@ nd6_dad_starttimer(struct dadq *dp, int
static void
nd6_dad_destroytimer(struct dadq *dp)
{
+ struct ifaddr *ifa;
TAILQ_REMOVE(&dadq, dp, dad_list);
/* Request the timer to destroy dp. */
+ ifa = dp->dad_ifa;
dp->dad_ifa = NULL;
+ ifafree(ifa);
callout_reset(&dp->dad_timer_ch, 0,
(void (*)(void *))nd6_dad_timer, dp);
}
@@ -1212,8 +1215,6 @@ nd6_dad_stop(struct ifaddr *ifa)
nd6_dad_destroytimer(dp);
mutex_exit(&nd6_dad_lock);
-
- ifafree(ifa);
}
static void
@@ -1230,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;
}
@@ -1315,6 +1311,9 @@ nd6_dad_timer(struct dadq *dp)
}
}
done:
+ if (duplicate)
+ nd6_dad_duplicated(dp);
+
mutex_exit(&nd6_dad_lock);
if (need_free) {
@@ -1324,29 +1323,22 @@ done:
ifafree(ifa);
}
- if (duplicate)
- nd6_dad_duplicated(ifa);
-
SOFTNET_KERNEL_UNLOCK_UNLESS_NET_MPSAFE();
}
static void
-nd6_dad_duplicated(struct ifaddr *ifa)
+nd6_dad_duplicated(struct dadq *dp)
{
- struct in6_ifaddr *ia = (struct in6_ifaddr *)ifa;
+ struct ifaddr *ifa = dp->dad_ifa;
+ struct in6_ifaddr *ia;
struct ifnet *ifp;
- struct dadq *dp;
char ip6buf[INET6_ADDRSTRLEN];
- mutex_enter(&nd6_dad_lock);
- dp = nd6_dad_find(ifa);
- if (dp == NULL) {
- mutex_exit(&nd6_dad_lock);
- /* DAD seems to be stopping, so do nothing. */
- return;
- }
+ KASSERT(mutex_owned(&nd6_dad_lock));
+ KASSERT(ifa != NULL);
ifp = ifa->ifa_ifp;
+ ia = (struct in6_ifaddr *)ifa;
log(LOG_ERR, "%s: DAD detected duplicate IPv6 address %s: "
"NS in/out=%d/%d, NA in=%d\n",
if_name(ifp), IN6_PRINT(ip6buf, &ia->ia_addr.sin6_addr),
@@ -1399,10 +1391,6 @@ nd6_dad_duplicated(struct ifaddr *ifa)
/* We are done with DAD, with duplicated address found. (failure) */
nd6_dad_destroytimer(dp);
-
- mutex_exit(&nd6_dad_lock);
-
- ifafree(ifa);
}
static void
@@ -1467,9 +1455,7 @@ nd6_dad_ns_input(struct ifaddr *ifa)
/* XXX more checks for loopback situation - see nd6_dad_timer too */
if (duplicate) {
- dp = NULL; /* will be freed in nd6_dad_duplicated() */
- mutex_exit(&nd6_dad_lock);
- nd6_dad_duplicated(ifa);
+ nd6_dad_duplicated(dp);
} else {
/*
* not sure if I got a duplicate.
@@ -1477,8 +1463,8 @@ nd6_dad_ns_input(struct ifaddr *ifa)
*/
if (dp)
dp->dad_ns_icount++;
- mutex_exit(&nd6_dad_lock);
}
+ mutex_exit(&nd6_dad_lock);
}
static void
@@ -1486,15 +1472,15 @@ nd6_dad_na_input(struct ifaddr *ifa)
{
struct dadq *dp;
- if (ifa == NULL)
- panic("ifa == NULL in nd6_dad_na_input");
+ KASSERT(ifa != NULL);
mutex_enter(&nd6_dad_lock);
dp = nd6_dad_find(ifa);
if (dp)
dp->dad_na_icount++;
- mutex_exit(&nd6_dad_lock);
/* remove the address. */
- nd6_dad_duplicated(ifa);
+ nd6_dad_duplicated(dp);
+
+ mutex_exit(&nd6_dad_lock);
}
