svn commit: r275593 - head/sys/netinet6
Mark Johnston
markj at FreeBSD.org
Mon Dec 8 04:44:41 UTC 2014
Author: markj
Date: Mon Dec 8 04:44:40 2014
New Revision: 275593
URL: https://svnweb.freebsd.org/changeset/base/275593
Log:
Add refcounting to IPv6 DAD objects and simplify the DAD code to fix a
number of races which could cause double frees or use-after-frees when
performing DAD on an address. In particular, an IPv6 address can now only be
marked as a duplicate from the DAD callout.
Differential Revision: https://reviews.freebsd.org/D1258
Reviewed by: ae, hrs
Reported by: rstone
MFC after: 1 month
Modified:
head/sys/netinet6/nd6.c
head/sys/netinet6/nd6.h
head/sys/netinet6/nd6_nbr.c
Modified: head/sys/netinet6/nd6.c
==============================================================================
--- head/sys/netinet6/nd6.c Mon Dec 8 04:35:34 2014 (r275592)
+++ head/sys/netinet6/nd6.c Mon Dec 8 04:44:40 2014 (r275593)
@@ -153,6 +153,8 @@ nd6_init(void)
callout_init(&V_nd6_slowtimo_ch, 0);
callout_reset(&V_nd6_slowtimo_ch, ND6_SLOWTIMER_INTERVAL * hz,
nd6_slowtimo, curvnet);
+
+ nd6_dad_init();
}
#ifdef VIMAGE
Modified: head/sys/netinet6/nd6.h
==============================================================================
--- head/sys/netinet6/nd6.h Mon Dec 8 04:35:34 2014 (r275592)
+++ head/sys/netinet6/nd6.h Mon Dec 8 04:44:40 2014 (r275593)
@@ -428,6 +428,7 @@ void nd6_ns_input(struct mbuf *, int, in
void nd6_ns_output(struct ifnet *, const struct in6_addr *,
const struct in6_addr *, struct llentry *, int);
caddr_t nd6_ifptomac(struct ifnet *);
+void nd6_dad_init(void);
void nd6_dad_start(struct ifaddr *, int);
void nd6_dad_stop(struct ifaddr *);
Modified: head/sys/netinet6/nd6_nbr.c
==============================================================================
--- head/sys/netinet6/nd6_nbr.c Mon Dec 8 04:35:34 2014 (r275592)
+++ head/sys/netinet6/nd6_nbr.c Mon Dec 8 04:44:40 2014 (r275593)
@@ -51,6 +51,7 @@ __FBSDID("$FreeBSD$");
#include <sys/syslog.h>
#include <sys/queue.h>
#include <sys/callout.h>
+#include <sys/refcount.h>
#include <net/if.h>
#include <net/if_types.h>
@@ -81,6 +82,7 @@ struct dadq;
static struct dadq *nd6_dad_find(struct ifaddr *);
static void nd6_dad_add(struct dadq *dp);
static void nd6_dad_del(struct dadq *dp);
+static void nd6_dad_rele(struct dadq *);
static void nd6_dad_starttimer(struct dadq *, int);
static void nd6_dad_stoptimer(struct dadq *);
static void nd6_dad_timer(struct dadq *);
@@ -1167,6 +1169,7 @@ struct dadq {
int dad_na_icount;
struct callout dad_timer_ch;
struct vnet *dad_vnet;
+ u_int dad_refcnt;
};
static VNET_DEFINE(TAILQ_HEAD(, dadq), dadq);
@@ -1174,9 +1177,6 @@ static VNET_DEFINE(struct rwlock, dad_rw
#define V_dadq VNET(dadq)
#define V_dad_rwlock VNET(dad_rwlock)
-#define DADQ_LOCK_INIT() rw_init(&V_dad_rwlock, "nd6 DAD queue")
-#define DADQ_LOCK_DESTROY() rw_destroy(&V_dad_rwlock)
-#define DADQ_LOCK_INITIALIZED() rw_initialized(&V_dad_rwlock)
#define DADQ_RLOCK() rw_rlock(&V_dad_rwlock)
#define DADQ_RUNLOCK() rw_runlock(&V_dad_rwlock)
#define DADQ_WLOCK() rw_wlock(&V_dad_rwlock)
@@ -1186,9 +1186,8 @@ static void
nd6_dad_add(struct dadq *dp)
{
- ifa_ref(dp->dad_ifa); /* just for safety */
DADQ_WLOCK();
- TAILQ_INSERT_TAIL(&V_dadq, (struct dadq *)dp, dad_list);
+ TAILQ_INSERT_TAIL(&V_dadq, dp, dad_list);
DADQ_WUNLOCK();
}
@@ -1196,10 +1195,10 @@ static void
nd6_dad_del(struct dadq *dp)
{
- ifa_free(dp->dad_ifa);
DADQ_WLOCK();
- TAILQ_REMOVE(&V_dadq, (struct dadq *)dp, dad_list);
+ TAILQ_REMOVE(&V_dadq, dp, dad_list);
DADQ_WUNLOCK();
+ nd6_dad_rele(dp);
}
static struct dadq *
@@ -1209,8 +1208,10 @@ nd6_dad_find(struct ifaddr *ifa)
DADQ_RLOCK();
TAILQ_FOREACH(dp, &V_dadq, dad_list)
- if (dp->dad_ifa == ifa)
+ if (dp->dad_ifa == ifa) {
+ refcount_acquire(&dp->dad_refcnt);
break;
+ }
DADQ_RUNLOCK();
return (dp);
@@ -1228,7 +1229,25 @@ static void
nd6_dad_stoptimer(struct dadq *dp)
{
- callout_stop(&dp->dad_timer_ch);
+ callout_drain(&dp->dad_timer_ch);
+}
+
+static void
+nd6_dad_rele(struct dadq *dp)
+{
+
+ if (refcount_release(&dp->dad_refcnt)) {
+ ifa_free(dp->dad_ifa);
+ free(dp, M_IP6NDP);
+ }
+}
+
+void
+nd6_dad_init(void)
+{
+
+ rw_init(&V_dad_rwlock, "nd6 DAD queue");
+ TAILQ_INIT(&V_dadq);
}
/*
@@ -1241,11 +1260,6 @@ nd6_dad_start(struct ifaddr *ifa, int de
struct dadq *dp;
char ip6buf[INET6_ADDRSTRLEN];
- if (DADQ_LOCK_INITIALIZED() == 0) {
- DADQ_LOCK_INIT();
- TAILQ_INIT(&V_dadq);
- }
-
/*
* If we don't need DAD, don't do it.
* There are several cases:
@@ -1275,12 +1289,13 @@ nd6_dad_start(struct ifaddr *ifa, int de
}
if (ND_IFINFO(ifa->ifa_ifp)->flags & ND6_IFF_IFDISABLED)
return;
- if (nd6_dad_find(ifa) != NULL) {
+ if ((dp = nd6_dad_find(ifa)) != NULL) {
/* DAD already in progress */
+ nd6_dad_rele(dp);
return;
}
- dp = malloc(sizeof(*dp), M_IP6NDP, M_NOWAIT);
+ dp = malloc(sizeof(*dp), M_IP6NDP, M_NOWAIT | M_ZERO);
if (dp == NULL) {
log(LOG_ERR, "nd6_dad_start: memory allocation failed for "
"%s(%s)\n",
@@ -1288,7 +1303,6 @@ nd6_dad_start(struct ifaddr *ifa, int de
ifa->ifa_ifp ? if_name(ifa->ifa_ifp) : "???");
return;
}
- bzero(dp, sizeof(*dp));
callout_init(&dp->dad_timer_ch, 0);
#ifdef VIMAGE
dp->dad_vnet = curvnet;
@@ -1303,9 +1317,11 @@ nd6_dad_start(struct ifaddr *ifa, int de
* (re)initialization.
*/
dp->dad_ifa = ifa;
+ ifa_ref(dp->dad_ifa);
dp->dad_count = V_ip6_dad_count;
dp->dad_ns_icount = dp->dad_na_icount = 0;
dp->dad_ns_ocount = dp->dad_ns_tcount = 0;
+ refcount_init(&dp->dad_refcnt, 1);
nd6_dad_add(dp);
if (delay == 0) {
nd6_dad_ns_output(dp, ifa);
@@ -1324,8 +1340,6 @@ nd6_dad_stop(struct ifaddr *ifa)
{
struct dadq *dp;
- if (DADQ_LOCK_INITIALIZED() == 0)
- return;
dp = nd6_dad_find(ifa);
if (!dp) {
/* DAD wasn't started yet */
@@ -1334,8 +1348,16 @@ nd6_dad_stop(struct ifaddr *ifa)
nd6_dad_stoptimer(dp);
+ /*
+ * The DAD queue entry may have been removed by nd6_dad_timer() while
+ * we were waiting for it to stop, so re-do the lookup.
+ */
+ nd6_dad_rele(dp);
+ if (nd6_dad_find(ifa) == NULL)
+ return;
+
nd6_dad_del(dp);
- free(dp, M_IP6NDP);
+ nd6_dad_rele(dp);
}
static void
@@ -1350,42 +1372,34 @@ nd6_dad_timer(struct dadq *dp)
/* Sanity check */
if (ia == NULL) {
log(LOG_ERR, "nd6_dad_timer: called with null parameter\n");
- goto done;
+ goto err;
}
if (ND_IFINFO(ifp)->flags & ND6_IFF_IFDISABLED) {
/* Do not need DAD for ifdisabled interface. */
- TAILQ_REMOVE(&V_dadq, (struct dadq *)dp, dad_list);
log(LOG_ERR, "nd6_dad_timer: cancel DAD on %s because of "
"ND6_IFF_IFDISABLED.\n", ifp->if_xname);
- free(dp, M_IP6NDP);
- dp = NULL;
- ifa_free(ifa);
- goto done;
+ goto err;
}
if (ia->ia6_flags & IN6_IFF_DUPLICATED) {
log(LOG_ERR, "nd6_dad_timer: called with duplicated address "
"%s(%s)\n",
ip6_sprintf(ip6buf, &ia->ia_addr.sin6_addr),
ifa->ifa_ifp ? if_name(ifa->ifa_ifp) : "???");
- goto done;
+ goto err;
}
if ((ia->ia6_flags & IN6_IFF_TENTATIVE) == 0) {
log(LOG_ERR, "nd6_dad_timer: called with non-tentative address "
"%s(%s)\n",
ip6_sprintf(ip6buf, &ia->ia_addr.sin6_addr),
ifa->ifa_ifp ? if_name(ifa->ifa_ifp) : "???");
- goto done;
+ goto err;
}
/* timeouted with IFF_{RUNNING,UP} check */
if (dp->dad_ns_tcount > V_dad_maxtry) {
nd6log((LOG_INFO, "%s: could not run DAD, driver problem?\n",
if_name(ifa->ifa_ifp)));
-
- nd6_dad_del(dp);
- free(dp, M_IP6NDP);
- dp = NULL;
- goto done;
+ goto err;
}
/* Need more checks? */
@@ -1396,33 +1410,16 @@ nd6_dad_timer(struct dadq *dp)
nd6_dad_ns_output(dp, ifa);
nd6_dad_starttimer(dp,
(long)ND_IFINFO(ifa->ifa_ifp)->retrans * hz / 1000);
+ goto done;
} else {
/*
* We have transmitted sufficient number of DAD packets.
* See what we've got.
*/
- int duplicate;
-
- duplicate = 0;
-
- if (dp->dad_na_icount) {
- /*
- * the check is in nd6_dad_na_input(),
- * but just in case
- */
- duplicate++;
- }
-
- if (dp->dad_ns_icount) {
- /* We've seen NS, means DAD has failed. */
- duplicate++;
- }
-
- if (duplicate) {
- /* (*dp) will be freed in nd6_dad_duplicated() */
+ if (dp->dad_ns_icount > 0 || dp->dad_na_icount > 0)
+ /* We've seen NS or NA, means DAD has failed. */
nd6_dad_duplicated(ifa, dp);
- dp = NULL;
- } else {
+ else {
/*
* We are done with DAD. No NA came, no NS came.
* No duplicate address found. Check IFDISABLED flag
@@ -1436,18 +1433,15 @@ nd6_dad_timer(struct dadq *dp)
"%s: DAD complete for %s - no duplicates found\n",
if_name(ifa->ifa_ifp),
ip6_sprintf(ip6buf, &ia->ia_addr.sin6_addr)));
-
- nd6_dad_del(dp);
- free(dp, M_IP6NDP);
- dp = NULL;
}
}
-
+err:
+ nd6_dad_del(dp);
done:
CURVNET_RESTORE();
}
-void
+static void
nd6_dad_duplicated(struct ifaddr *ifa, struct dadq *dp)
{
struct in6_ifaddr *ia = (struct in6_ifaddr *)ifa;
@@ -1462,9 +1456,6 @@ nd6_dad_duplicated(struct ifaddr *ifa, s
ia->ia6_flags &= ~IN6_IFF_TENTATIVE;
ia->ia6_flags |= IN6_IFF_DUPLICATED;
- /* We are done with DAD, with duplicate address found. (failure) */
- nd6_dad_stoptimer(dp);
-
ifp = ifa->ifa_ifp;
log(LOG_ERR, "%s: DAD complete for %s - duplicate found\n",
if_name(ifp), ip6_sprintf(ip6buf, &ia->ia_addr.sin6_addr));
@@ -1505,9 +1496,6 @@ nd6_dad_duplicated(struct ifaddr *ifa, s
break;
}
}
-
- nd6_dad_del(dp);
- free(dp, M_IP6NDP);
}
static void
@@ -1535,7 +1523,6 @@ nd6_dad_ns_input(struct ifaddr *ifa)
struct ifnet *ifp;
const struct in6_addr *taddr6;
struct dadq *dp;
- int duplicate;
if (ifa == NULL)
panic("ifa == NULL in nd6_dad_ns_input");
@@ -1543,8 +1530,9 @@ nd6_dad_ns_input(struct ifaddr *ifa)
ia = (struct in6_ifaddr *)ifa;
ifp = ifa->ifa_ifp;
taddr6 = &ia->ia_addr.sin6_addr;
- duplicate = 0;
dp = nd6_dad_find(ifa);
+ if (dp == NULL)
+ return;
/* Quickhack - completely ignore DAD NS packets */
if (V_dad_ignore_ns) {
@@ -1556,26 +1544,10 @@ nd6_dad_ns_input(struct ifaddr *ifa)
return;
}
- /*
- * if I'm yet to start DAD, someone else started using this address
- * first. I have a duplicate and you win.
- */
- if (dp == NULL || dp->dad_ns_ocount == 0)
- duplicate++;
-
/* XXX more checks for loopback situation - see nd6_dad_timer too */
- if (duplicate) {
- nd6_dad_duplicated(ifa, dp);
- dp = NULL; /* will be freed in nd6_dad_duplicated() */
- } else {
- /*
- * not sure if I got a duplicate.
- * increment ns count and see what happens.
- */
- if (dp)
- dp->dad_ns_icount++;
- }
+ dp->dad_ns_icount++;
+ nd6_dad_rele(dp);
}
static void
@@ -1587,9 +1559,8 @@ nd6_dad_na_input(struct ifaddr *ifa)
panic("ifa == NULL in nd6_dad_na_input");
dp = nd6_dad_find(ifa);
- if (dp)
+ if (dp != NULL) {
dp->dad_na_icount++;
-
- /* remove the address. */
- nd6_dad_duplicated(ifa, dp);
+ nd6_dad_rele(dp);
+ }
}
More information about the svn-src-all
mailing list