d/freebsd-skq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Add refcounting to IPv6 DAD objects and simplify the DAD code to fix a

Browse Source 
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
This commit is contained in:
Mark Johnston 2014-12-08 04:44:40 +00:00
parent 38d120bc13
commit d6ad6a865a
3 changed files with 64 additions and 90 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
sys/netinet6/nd6.c
View File

@ -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

1
sys/netinet6/nd6.h
View File

@ -428,6 +428,7 @@ void nd6_ns_input(struct mbuf *, int, int);
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 *);

151
sys/netinet6/nd6_nbr.c
View File

@ -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_rwlock);
#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 delay)
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 delay)
}
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 delay)
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 delay)
* (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, struct dadq *dp)
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, struct dadq *dp)
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);
}
}