Serialize access to the IPv6 reassembly queue with a mutex.

This lets the code run without KERNEL_LOCK() and NET_LOCK().

In addition, this patch reorganizes a part of the code, to move
some of the memory release operations outside the critical section.

Input and OK mpi@, OK florian@, an earlier version OK bluhm@

1 files changed, 71 insertions, 83 deletions

diff --git a/sys/netinet6/frag6.c b/sys/netinet6/frag6.c
index 20bbcd1ce33..0e9b1ca3edf 100644
--- a/sys/netinet6/frag6.c
+++ b/sys/netinet6/frag6.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: frag6.c,v 1.78 2017/11/05 13:19:59 florian Exp $	*/
+/*	$OpenBSD: frag6.c,v 1.79 2017/11/07 16:47:07 visa Exp $	*/
 /*	$KAME: frag6.c,v 1.40 2002/05/27 21:40:31 itojun Exp $	*/
 
 /*
@@ -41,6 +41,7 @@
 #include <sys/kernel.h>
 #include <sys/syslog.h>
 #include <sys/pool.h>
+#include <sys/mutex.h>
 
 #include <net/if.h>
 #include <net/if_var.h>
@@ -53,67 +54,19 @@
 #include <netinet/icmp6.h>
 #include <netinet/ip.h>		/* for ECN definitions */
 
-void frag6_freef(struct ip6q *);
+/* Protects `frag6_queue', `frag6_nfragpackets' and `frag6_nfrags'. */
+struct mutex frag6_mutex = MUTEX_INITIALIZER(IPL_SOFTNET);
 
-static int ip6q_locked;
 u_int frag6_nfragpackets;
 u_int frag6_nfrags;
 TAILQ_HEAD(ip6q_head, ip6q) frag6_queue;	/* ip6 reassemble queue */
 
+void frag6_freef(struct ip6q *);
+void frag6_unlink(struct ip6q *, struct ip6q_head *);
+
 struct pool ip6af_pool;
 struct pool ip6q_pool;
 
-static __inline int ip6q_lock_try(void);
-static __inline void ip6q_unlock(void);
-
-static __inline int
-ip6q_lock_try(void)
-{
-	int s;
-
-	/* Use splvm() due to mbuf allocation. */
-	s = splvm();
-	if (ip6q_locked) {
-		splx(s);
-		return (0);
-	}
-	ip6q_locked = 1;
-	splx(s);
-	return (1);
-}
-
-static __inline void
-ip6q_unlock(void)
-{
-	int s;
-
-	s = splvm();
-	ip6q_locked = 0;
-	splx(s);
-}
-
-#ifdef DIAGNOSTIC
-#define	IP6Q_LOCK()							\
-do {									\
-	if (ip6q_lock_try() == 0) {					\
-		printf("%s:%d: ip6q already locked\n", __FILE__, __LINE__); \
-		panic("ip6q_lock");					\
-	}								\
-} while (0)
-#define	IP6Q_LOCK_CHECK()						\
-do {									\
-	if (ip6q_locked == 0) {						\
-		printf("%s:%d: ip6q lock not held\n", __FILE__, __LINE__); \
-		panic("ip6q lock check");				\
-	}								\
-} while (0)
-#else
-#define	IP6Q_LOCK()		(void) ip6q_lock_try()
-#define	IP6Q_LOCK_CHECK()	/* nothing */
-#endif
-
-#define	IP6Q_UNLOCK()		ip6q_unlock()
-
 /*
  * Initialise reassembly queue and pools.
  */
@@ -221,15 +174,17 @@ frag6_input(struct mbuf **mp, int *offp, int proto, int af)
 		return IPPROTO_DONE;
 	}
 
-	IP6Q_LOCK();
+	mtx_enter(&frag6_mutex);
 
 	/*
 	 * Enforce upper bound on number of fragments.
 	 * If maxfrag is 0, never accept fragments.
 	 * If maxfrag is -1, accept all fragments without limitation.
 	 */
-	if (ip6_maxfrags >= 0 && frag6_nfrags >= (u_int)ip6_maxfrags)
+	if (ip6_maxfrags >= 0 && frag6_nfrags >= (u_int)ip6_maxfrags) {
+		mtx_leave(&frag6_mutex);
 		goto dropfrag;
+	}
 
 	TAILQ_FOREACH(q6, &frag6_queue, ip6q_queue)
 		if (ip6f->ip6f_ident == q6->ip6q_ident &&
@@ -251,12 +206,16 @@ frag6_input(struct mbuf **mp, int *offp, int proto, int af)
 		 * limitation.
 		 */
 		if (ip6_maxfragpackets >= 0 &&
-		    frag6_nfragpackets >= (u_int)ip6_maxfragpackets)
+		    frag6_nfragpackets >= (u_int)ip6_maxfragpackets) {
+			mtx_leave(&frag6_mutex);
 			goto dropfrag;
+		}
 		frag6_nfragpackets++;
 		q6 = pool_get(&ip6q_pool, PR_NOWAIT | PR_ZERO);
-		if (q6 == NULL)
+		if (q6 == NULL) {
+			mtx_leave(&frag6_mutex);
 			goto dropfrag;
+		}
 
 		TAILQ_INSERT_HEAD(&frag6_queue, q6, ip6q_queue);
 
@@ -289,17 +248,17 @@ frag6_input(struct mbuf **mp, int *offp, int proto, int af)
 	if (q6->ip6q_unfrglen >= 0) {
 		/* The 1st fragment has already arrived. */
 		if (q6->ip6q_unfrglen + fragoff + frgpartlen > IPV6_MAXPACKET) {
+			mtx_leave(&frag6_mutex);
 			icmp6_error(m, ICMP6_PARAM_PROB, ICMP6_PARAMPROB_HEADER,
 			    offset - sizeof(struct ip6_frag) +
 			    offsetof(struct ip6_frag, ip6f_offlg));
-			IP6Q_UNLOCK();
 			return (IPPROTO_DONE);
 		}
 	} else if (fragoff + frgpartlen > IPV6_MAXPACKET) {
+		mtx_leave(&frag6_mutex);
 		icmp6_error(m, ICMP6_PARAM_PROB, ICMP6_PARAMPROB_HEADER,
 			    offset - sizeof(struct ip6_frag) +
 				offsetof(struct ip6_frag, ip6f_offlg));
-		IP6Q_UNLOCK();
 		return (IPPROTO_DONE);
 	}
 	/*
@@ -337,8 +296,10 @@ frag6_input(struct mbuf **mp, int *offp, int proto, int af)
 	}
 
 	ip6af = pool_get(&ip6af_pool, PR_NOWAIT | PR_ZERO);
-	if (ip6af == NULL)
+	if (ip6af == NULL) {
+		mtx_leave(&frag6_mutex);
 		goto dropfrag;
+	}
 	ip6af->ip6af_flow = ip6->ip6_flow;
 	ip6af->ip6af_mff = ip6f->ip6f_offlg & IP6F_MORE_FRAG;
 	ip6af->ip6af_off = fragoff;
@@ -361,6 +322,7 @@ frag6_input(struct mbuf **mp, int *offp, int proto, int af)
 	ecn0 = (ntohl(af6->ip6af_flow) >> 20) & IPTOS_ECN_MASK;
 	if (ecn == IPTOS_ECN_CE) {
 		if (ecn0 == IPTOS_ECN_NOTECT) {
+			mtx_leave(&frag6_mutex);
 			pool_put(&ip6af_pool, ip6af);
 			goto dropfrag;
 		}
@@ -368,6 +330,7 @@ frag6_input(struct mbuf **mp, int *offp, int proto, int af)
 			af6->ip6af_flow |= htonl(IPTOS_ECN_CE << 20);
 	}
 	if (ecn == IPTOS_ECN_NOTECT && ecn0 != IPTOS_ECN_NOTECT) {
+		mtx_leave(&frag6_mutex);
 		pool_put(&ip6af_pool, ip6af);
 		goto dropfrag;
 	}
@@ -397,7 +360,6 @@ frag6_input(struct mbuf **mp, int *offp, int proto, int af)
 			    i,
 			    inet_ntop(AF_INET6, &q6->ip6q_src, ip, sizeof(ip)));
 #endif
-			pool_put(&ip6af_pool, ip6af);
 			goto flushfrags;
 		}
 	}
@@ -411,7 +373,6 @@ frag6_input(struct mbuf **mp, int *offp, int proto, int af)
 			    i,
 			    inet_ntop(AF_INET6, &q6->ip6q_src, ip, sizeof(ip)));
 #endif
-			pool_put(&ip6af_pool, ip6af);
 			goto flushfrags;
 		}
 	}
@@ -440,13 +401,13 @@ frag6_input(struct mbuf **mp, int *offp, int proto, int af)
 	    af6 != NULL;
 	    paf6 = af6, af6 = LIST_NEXT(af6, ip6af_list)) {
 		if (af6->ip6af_off != next) {
-			IP6Q_UNLOCK();
+			mtx_leave(&frag6_mutex);
 			return IPPROTO_DONE;
 		}
 		next += af6->ip6af_frglen;
 	}
 	if (paf6->ip6af_mff) {
-		IP6Q_UNLOCK();
+		mtx_leave(&frag6_mutex);
 		return IPPROTO_DONE;
 	}
 
@@ -478,8 +439,9 @@ frag6_input(struct mbuf **mp, int *offp, int proto, int af)
 	if (frag6_deletefraghdr(m, offset) != 0) {
 		TAILQ_REMOVE(&frag6_queue, q6, ip6q_queue);
 		frag6_nfrags -= q6->ip6q_nfrag;
-		pool_put(&ip6q_pool, q6);
 		frag6_nfragpackets--;
+		mtx_leave(&frag6_mutex);
+		pool_put(&ip6q_pool, q6);
 		goto dropfrag;
 	}
 
@@ -493,9 +455,12 @@ frag6_input(struct mbuf **mp, int *offp, int proto, int af)
 
 	TAILQ_REMOVE(&frag6_queue, q6, ip6q_queue);
 	frag6_nfrags -= q6->ip6q_nfrag;
-	pool_put(&ip6q_pool, q6);
 	frag6_nfragpackets--;
 
+	mtx_leave(&frag6_mutex);
+
+	pool_put(&ip6q_pool, q6);
+
 	if (m->m_flags & M_PKTHDR) { /* Isn't it always true? */
 		int plen = 0;
 		for (t = m; t; t = t->m_next)
@@ -512,25 +477,30 @@ frag6_input(struct mbuf **mp, int *offp, int proto, int af)
 	*mp = m;
 	*offp = offset;
 
-	IP6Q_UNLOCK();
 	return nxt;
 
  flushfrags:
+	TAILQ_REMOVE(&frag6_queue, q6, ip6q_queue);
+	frag6_nfrags -= q6->ip6q_nfrag;
+	frag6_nfragpackets--;
+
+	mtx_leave(&frag6_mutex);
+
+	pool_put(&ip6af_pool, ip6af);
+
 	while ((af6 = LIST_FIRST(&q6->ip6q_asfrag)) != NULL) {
 		LIST_REMOVE(af6, ip6af_list);
 		m_freem(af6->ip6af_m);
 		pool_put(&ip6af_pool, af6);
 	}
-	ip6stat_add(ip6s_fragdropped, q6->ip6q_nfrag);
-	TAILQ_REMOVE(&frag6_queue, q6, ip6q_queue);
-	frag6_nfrags -= q6->ip6q_nfrag;
+	ip6stat_add(ip6s_fragdropped, q6->ip6q_nfrag + 1);
 	pool_put(&ip6q_pool, q6);
-	frag6_nfragpackets--;
+	m_freem(m);
+	return IPPROTO_DONE;
 
  dropfrag:
 	ip6stat_inc(ip6s_fragdropped);
 	m_freem(m);
-	IP6Q_UNLOCK();
 	return IPPROTO_DONE;
 }
 
@@ -561,14 +531,13 @@ frag6_deletefraghdr(struct mbuf *m, int offset)
 /*
  * Free a fragment reassembly header and all
  * associated datagrams.
+ * The header must not be in any queue.
  */
 void
 frag6_freef(struct ip6q *q6)
 {
 	struct ip6asfrag *af6;
 
-	IP6Q_LOCK_CHECK();
-
 	while ((af6 = LIST_FIRST(&q6->ip6q_asfrag)) != NULL) {
 		struct mbuf *m = af6->ip6af_m;
 
@@ -594,9 +563,21 @@ frag6_freef(struct ip6q *q6)
 			m_freem(m);
 		pool_put(&ip6af_pool, af6);
 	}
+	pool_put(&ip6q_pool, q6);
+}
+
+/*
+ * Unlinks a fragment reassembly header from the reassembly queue
+ * and inserts it into a given remove queue.
+ */
+void
+frag6_unlink(struct ip6q *q6, struct ip6q_head *rmq6)
+{
+	MUTEX_ASSERT_LOCKED(&frag6_mutex);
+
 	TAILQ_REMOVE(&frag6_queue, q6, ip6q_queue);
+	TAILQ_INSERT_HEAD(rmq6, q6, ip6q_queue);
 	frag6_nfrags -= q6->ip6q_nfrag;
-	pool_put(&ip6q_pool, q6);
 	frag6_nfragpackets--;
 }
 
@@ -608,16 +589,19 @@ frag6_freef(struct ip6q *q6)
 void
 frag6_slowtimo(void)
 {
+	struct ip6q_head rmq6;
 	struct ip6q *q6, *nq6;
 
-	NET_LOCK();
+	TAILQ_INIT(&rmq6);
+
+	mtx_enter(&frag6_mutex);
 
-	IP6Q_LOCK();
-	TAILQ_FOREACH_SAFE(q6, &frag6_queue, ip6q_queue, nq6)
+	TAILQ_FOREACH_SAFE(q6, &frag6_queue, ip6q_queue, nq6) {
 		if (--q6->ip6q_ttl == 0) {
 			ip6stat_inc(ip6s_fragtimeout);
-			frag6_freef(q6);
+			frag6_unlink(q6, &rmq6);
 		}
+	}
 
 	/*
 	 * If we are over the maximum number of fragments
@@ -627,9 +611,13 @@ frag6_slowtimo(void)
 	while (frag6_nfragpackets > (u_int)ip6_maxfragpackets &&
 	    !TAILQ_EMPTY(&frag6_queue)) {
 		ip6stat_inc(ip6s_fragoverflow);
-		frag6_freef(TAILQ_LAST(&frag6_queue, ip6q_head));
+		frag6_unlink(TAILQ_LAST(&frag6_queue, ip6q_head), &rmq6);
 	}
-	IP6Q_UNLOCK();
 
-	NET_UNLOCK();
+	mtx_leave(&frag6_mutex);
+
+	while ((q6 = TAILQ_FIRST(&rmq6)) != NULL) {
+		TAILQ_REMOVE(&rmq6, q6, ip6q_queue);
+		frag6_freef(q6);
+	}
 }