dummynet: Fix schedlist and aqmlist locking

These are global (i.e. shared across vnets) structures, so we need
global lock to protect them.  However, we look up entries in these lists
(find_aqm_type(), find_sched_type()) and return them. We must ensure
that the returned structures cannot go away while we are using them.

Resolve this by using NET_EPOCH(). The structures can be safely accessed
under it, and we postpone their cleanup until we're sure they're no
longer used.

MFC after:	2 weeks
Sponsored by:	Rubicon Communications, LLC ("Netgate")
Differential Revision:	https://reviews.freebsd.org/D30381

(cherry picked from commit 51d73df18e4d120f6f062062c18efae3ed5193a6)

4 files changed, 98 insertions, 38 deletions

diff --git a/sys/netpfil/ipfw/dn_aqm.h b/sys/netpfil/ipfw/dn_aqm.h
index b0eaf2ecfc8a..cfa1c266c7c8 100644
--- a/sys/netpfil/ipfw/dn_aqm.h
+++ b/sys/netpfil/ipfw/dn_aqm.h
@@ -36,6 +36,8 @@
 #ifndef _IP_DN_AQM_H
 #define _IP_DN_AQM_H
 
+#include <sys/ck.h>
+
 /* NOW is the current time in millisecond*/
 #define NOW ((V_dn_cfg.curr_time * tick) / 1000)
 
@@ -107,7 +109,7 @@ typedef int32_t aqm_stime_t;
 
 	int	ref_count; /*Number of queues instances in the system */
 	int	cfg_ref_count;	/*Number of AQM instances in the system */
-	SLIST_ENTRY (dn_aqm) next; /* Next AQM in the list */
+	CK_LIST_ENTRY(dn_aqm) next; /* Next AQM in the list */
 };
 
 /* Helper function to update queue and scheduler statistics.
diff --git a/sys/netpfil/ipfw/dn_sched.h b/sys/netpfil/ipfw/dn_sched.h
index 1aa885ce3ccf..5c506c1d30ac 100644
--- a/sys/netpfil/ipfw/dn_sched.h
+++ b/sys/netpfil/ipfw/dn_sched.h
@@ -35,6 +35,8 @@
 #ifndef _DN_SCHED_H
 #define _DN_SCHED_H
 
+#include <sys/ck.h>
+
 #define	DN_MULTIQUEUE	0x01
 /*
  * Descriptor for a scheduling algorithm.
@@ -141,7 +143,7 @@ struct dn_alg {
 
 	/* run-time fields */
 	int ref_count;      /* XXX number of instances in the system */
-	SLIST_ENTRY(dn_alg) next; /* Next scheduler in the list */
+	CK_LIST_ENTRY(dn_alg) next; /* Next scheduler in the list */
 };
 
 /* MSVC does not support initializers so we need this ugly macro */
diff --git a/sys/netpfil/ipfw/ip_dn_glue.c b/sys/netpfil/ipfw/ip_dn_glue.c
index 83f26cb23680..e035fedaaf91 100644
--- a/sys/netpfil/ipfw/ip_dn_glue.c
+++ b/sys/netpfil/ipfw/ip_dn_glue.c
@@ -814,7 +814,11 @@ ip_dummynet_compat(struct sockopt *sopt)
 		break;
 
 	case IP_DUMMYNET_CONFIGURE:
-		v = malloc(len, M_TEMP, M_WAITOK);
+		v = malloc(len, M_TEMP, M_NOWAIT);
+		if (v == NULL) {
+			error = ENOMEM;
+			break;
+		}
 		error = sooptcopyin(sopt, v, len, len);
 		if (error)
 			break;
diff --git a/sys/netpfil/ipfw/ip_dummynet.c b/sys/netpfil/ipfw/ip_dummynet.c
index 3abc78fc1410..b03ad93041bd 100644
--- a/sys/netpfil/ipfw/ip_dummynet.c
+++ b/sys/netpfil/ipfw/ip_dummynet.c
@@ -44,6 +44,7 @@ __FBSDID("$FreeBSD$");
 #include "opt_inet6.h"
 
 #include <sys/param.h>
+#include <sys/ck.h>
 #include <sys/systm.h>
 #include <sys/malloc.h>
 #include <sys/mbuf.h>
@@ -94,9 +95,10 @@ static struct task	dn_task;
 static struct taskqueue	*dn_tq = NULL;
 
 /* global scheduler list */
-struct dn_alg_head	schedlist;
+struct mtx		sched_mtx;
+CK_LIST_HEAD(, dn_alg)	schedlist;
 #ifdef NEW_AQM
-struct dn_aqm_head	aqmlist;	/* list of AQMs */
+CK_LIST_HEAD(, dn_aqm)	aqmlist;	/* list of AQMs */
 #endif
 
 static void
@@ -125,7 +127,9 @@ find_aqm_type(int type, char *name)
 {
 	struct dn_aqm *d;
 
-	SLIST_FOREACH(d, &aqmlist, next) {
+	NET_EPOCH_ASSERT();
+
+	CK_LIST_FOREACH(d, &aqmlist, next) {
 		if (d->type == type || (name && !strcasecmp(d->name, name)))
 			return d;
 	}
@@ -139,7 +143,9 @@ find_sched_type(int type, char *name)
 {
 	struct dn_alg *d;
 
-	SLIST_FOREACH(d, &schedlist, next) {
+	NET_EPOCH_ASSERT();
+
+	CK_LIST_FOREACH(d, &schedlist, next) {
 		if (d->type == type || (name && !strcasecmp(d->name, name)))
 			return d;
 	}
@@ -1355,7 +1361,7 @@ get_aqm_parms(struct sockopt *sopt)
 		err = EINVAL;
 		return err;
 	}
-	ep = malloc(l, M_DUMMYNET, M_WAITOK);
+	ep = malloc(l, M_DUMMYNET, M_NOWAIT);
 	if(!ep) {
 		err = ENOMEM ;
 		return err;
@@ -1410,7 +1416,7 @@ get_sched_parms(struct sockopt *sopt)
 		err = EINVAL;
 		return err;
 	}
-	ep = malloc(l, M_DUMMYNET, M_WAITOK);
+	ep = malloc(l, M_DUMMYNET, M_NOWAIT);
 	if(!ep) {
 		err = ENOMEM ;
 		return err;
@@ -1455,6 +1461,8 @@ config_aqm(struct dn_fsk *fs, struct  dn_extra_parms *ep, int busy)
 {
 	int err = 0;
 
+	NET_EPOCH_ASSERT();
+
 	do {
 		/* no configurations */
 		if (!ep) {
@@ -1614,7 +1622,7 @@ config_fs(struct dn_fs *nfs, struct dn_id *arg, int locked)
 #ifdef NEW_AQM
 	ep = NULL;
 	if (arg != NULL) {
-		ep = malloc(sizeof(*ep), M_TEMP, locked ? M_NOWAIT : M_WAITOK);
+		ep = malloc(sizeof(*ep), M_TEMP, M_NOWAIT);
 		if (ep == NULL)
 			return (NULL);
 		memcpy(ep, arg, sizeof(*ep));
@@ -1727,6 +1735,8 @@ config_sched(struct dn_sch *_nsch, struct dn_id *arg)
 	int pipe_cmd;
 	int err = ENOMEM;
 
+	NET_EPOCH_ASSERT();
+
 	a.sch = _nsch;
 	if (a.sch->oid.len != sizeof(*a.sch)) {
 		D("bad sched len %d", a.sch->oid.len);
@@ -2070,34 +2080,53 @@ do_config(void *p, int l)
 			DN_BH_WUNLOCK();
 			break;
 		case DN_TEXT:	/* store argument of next block */
-			if (arg != NULL)
-				free(arg, M_TEMP);
-			arg = malloc(o.len, M_TEMP, M_WAITOK);
+			free(arg, M_TEMP);
+			arg = malloc(o.len, M_TEMP, M_NOWAIT);
+			if (arg == NULL) {
+				err = ENOMEM;
+				break;
+			}
 			memcpy(arg, (char *)p + off, o.len);
 			break;
 		case DN_LINK:
 			if (dn == NULL)
-				dn = malloc(sizeof(*dn), M_TEMP, M_WAITOK);
+				dn = malloc(sizeof(*dn), M_TEMP, M_NOWAIT);
+			if (dn == NULL) {
+				err = ENOMEM;
+				break;
+			}
 			memcpy(&dn->link, (char *)p + off, sizeof(dn->link));
 			err = config_link(&dn->link, arg);
 			break;
 		case DN_PROFILE:
 			if (dn == NULL)
-				dn = malloc(sizeof(*dn), M_TEMP, M_WAITOK);
+				dn = malloc(sizeof(*dn), M_TEMP, M_NOWAIT);
+			if (dn == NULL) {
+				err = ENOMEM;
+				break;
+			}
 			memcpy(&dn->profile, (char *)p + off,
 			    sizeof(dn->profile));
 			err = config_profile(&dn->profile, arg);
 			break;
 		case DN_SCH:
 			if (dn == NULL)
-				dn = malloc(sizeof(*dn), M_TEMP, M_WAITOK);
+				dn = malloc(sizeof(*dn), M_TEMP, M_NOWAIT);
+			if (dn == NULL) {
+				err = ENOMEM;
+				break;
+			}
 			memcpy(&dn->sched, (char *)p + off,
 			    sizeof(dn->sched));
 			err = config_sched(&dn->sched, arg);
 			break;
 		case DN_FS:
 			if (dn == NULL)
-				dn = malloc(sizeof(*dn), M_TEMP, M_WAITOK);
+				dn = malloc(sizeof(*dn), M_TEMP, M_NOWAIT);
+			if (dn == NULL) {
+				err = ENOMEM;
+				break;
+			}
 			memcpy(&dn->fs, (char *)p + off, sizeof(dn->fs));
 			err = (NULL == config_fs(&dn->fs, arg, 0));
 			break;
@@ -2230,7 +2259,11 @@ dummynet_get(struct sockopt *sopt, void **compat)
 #endif
 		if (l > sizeof(r)) {
 			/* request larger than default, allocate buffer */
-			cmd = malloc(l,  M_DUMMYNET, M_WAITOK);
+			cmd = malloc(l,  M_DUMMYNET, M_NOWAIT);
+			if (cmd == NULL) {
+				error = ENOMEM;
+				goto done;
+			}
 			error = sooptcopyin(sopt, cmd, l, l);
 			sopt->sopt_valsize = sopt_valsize;
 			if (error)
@@ -2298,7 +2331,7 @@ dummynet_get(struct sockopt *sopt, void **compat)
 			break;
 
 		have = need;
-		start = malloc(have, M_DUMMYNET, M_WAITOK | M_ZERO);
+		start = malloc(have, M_DUMMYNET, M_NOWAIT | M_ZERO);
 	}
 
 	if (start == NULL) {
@@ -2461,6 +2494,7 @@ dn_drain_queue(void)
 static int
 ip_dn_ctl(struct sockopt *sopt)
 {
+	struct epoch_tracker et;
 	void *p = NULL;
 	int error, l;
 
@@ -2475,6 +2509,8 @@ ip_dn_ctl(struct sockopt *sopt)
 			return (error);
 	}
 
+	NET_EPOCH_ENTER(et);
+
 	switch (sopt->sopt_name) {
 	default :
 		D("dummynet: unknown option %d", sopt->sopt_name);
@@ -2499,7 +2535,11 @@ ip_dn_ctl(struct sockopt *sopt)
 			D("argument len %d invalid", l);
 			break;
 		}
-		p = malloc(l, M_TEMP, M_WAITOK); // XXX can it fail ?
+		p = malloc(l, M_TEMP, M_NOWAIT);
+		if (p == NULL) {
+			error = ENOMEM;
+			break;
+		}
 		error = sooptcopyin(sopt, p, l, l);
 		if (error)
 			break ;
@@ -2510,6 +2550,8 @@ ip_dn_ctl(struct sockopt *sopt)
 	if (p != NULL)
 		free(p, M_TEMP);
 
+	NET_EPOCH_EXIT(et);
+
 	return error ;
 }
 
@@ -2578,13 +2620,16 @@ ip_dn_init(void)
 {
 	if (dn_tasks_started)
 		return;
+
+	mtx_init(&sched_mtx, "dn_sched", NULL, MTX_DEF);
+
 	dn_tasks_started = 1;
 	TASK_INIT(&dn_task, 0, dummynet_task, NULL);
 	dn_tq = taskqueue_create_fast("dummynet", M_WAITOK,
 	    taskqueue_thread_enqueue, &dn_tq);
 	taskqueue_start_threads(&dn_tq, 1, PI_NET, "dummynet");
 
-	SLIST_INIT(&schedlist);
+	CK_LIST_INIT(&schedlist);
 	callout_init(&dn_timeout, 1);
 	dn_reschedule();
 }
@@ -2644,16 +2689,16 @@ load_dn_sched(struct dn_alg *d)
 	}
 
 	/* Search if scheduler already exists */
-	DN_BH_WLOCK();
-	SLIST_FOREACH(s, &schedlist, next) {
+	mtx_lock(&sched_mtx);
+	CK_LIST_FOREACH(s, &schedlist, next) {
 		if (strcmp(s->name, d->name) == 0) {
 			D("%s already loaded", d->name);
 			break; /* scheduler already exists */
 		}
 	}
 	if (s == NULL)
-		SLIST_INSERT_HEAD(&schedlist, d, next);
-	DN_BH_WUNLOCK();
+		CK_LIST_INSERT_HEAD(&schedlist, d, next);
+	mtx_unlock(&sched_mtx);
 	D("dn_sched %s %sloaded", d->name, s ? "not ":"");
 	return s ? 1 : 0;
 }
@@ -2666,17 +2711,18 @@ unload_dn_sched(struct dn_alg *s)
 
 	ND("called for %s", s->name);
 
-	DN_BH_WLOCK();
-	SLIST_FOREACH_SAFE(r, &schedlist, next, tmp) {
+	mtx_lock(&sched_mtx);
+	CK_LIST_FOREACH_SAFE(r, &schedlist, next, tmp) {
 		if (strcmp(s->name, r->name) != 0)
 			continue;
 		ND("ref_count = %d", r->ref_count);
 		err = (r->ref_count != 0) ? EBUSY : 0;
 		if (err == 0)
-			SLIST_REMOVE(&schedlist, r, dn_alg, next);
+			CK_LIST_REMOVE(r, next);
 		break;
 	}
-	DN_BH_WUNLOCK();
+	mtx_unlock(&sched_mtx);
+	NET_EPOCH_WAIT();
 	D("dn_sched %s %sunloaded", s->name, err ? "not ":"");
 	return err;
 }
@@ -2736,17 +2782,20 @@ load_dn_aqm(struct dn_aqm *d)
 		return 1;
 	}
 
+	mtx_lock(&sched_mtx);
+
 	/* Search if AQM already exists */
-	DN_BH_WLOCK(); /* XXX Global lock? */
-	SLIST_FOREACH(aqm, &aqmlist, next) {
+	CK_LIST_FOREACH(aqm, &aqmlist, next) {
 		if (strcmp(aqm->name, d->name) == 0) {
 			D("%s already loaded", d->name);
 			break; /* AQM already exists */
 		}
 	}
 	if (aqm == NULL)
-		SLIST_INSERT_HEAD(&aqmlist, d, next);
-	DN_BH_WUNLOCK();
+		CK_LIST_INSERT_HEAD(&aqmlist, d, next);
+
+	mtx_unlock(&sched_mtx);
+
 	D("dn_aqm %s %sloaded", d->name, aqm ? "not ":"");
 	return aqm ? 1 : 0;
 }
@@ -2775,21 +2824,24 @@ unload_dn_aqm(struct dn_aqm *aqm)
 	err = 0;
 	ND("called for %s", aqm->name);
 
-	DN_BH_WLOCK();
-
 	/* clean up AQM status and deconfig flowset */
 	dn_ht_scan(V_dn_cfg.fshash, fs_cleanup, &aqm->type);
 
-	SLIST_FOREACH_SAFE(r, &aqmlist, next, tmp) {
+	mtx_lock(&sched_mtx);
+
+	CK_LIST_FOREACH_SAFE(r, &aqmlist, next, tmp) {
 		if (strcmp(aqm->name, r->name) != 0)
 			continue;
 		ND("ref_count = %d", r->ref_count);
 		err = (r->ref_count != 0 || r->cfg_ref_count != 0) ? EBUSY : 0;
 		if (err == 0)
-			SLIST_REMOVE(&aqmlist, r, dn_aqm, next);
+			CK_LIST_REMOVE(r, next);
 		break;
 	}
-	DN_BH_WUNLOCK();
+
+	mtx_unlock(&sched_mtx);
+	NET_EPOCH_WAIT();
+
 	D("%s %sunloaded", aqm->name, err ? "not ":"");
 	if (err)
 		D("ref_count=%d, cfg_ref_count=%d", r->ref_count, r->cfg_ref_count);