Module Name:    src
Committed By:   thorpej
Date:           Wed Nov 15 02:19:00 UTC 2023

Modified Files:
        src/sys/altq [thorpej-ifq]: altq_cdnr.c altq_subr.c
        src/sys/net [thorpej-ifq]: if.c if.h

Log Message:
Protect the ALTQ state that's exposed to the ifqueue if the ifq->ifq_lock.
This requires exposing some implementation details to ALTQ, which is guarded
by an __IFQ_PRIVATE define.


To generate a diff of this commit:
cvs rdiff -u -r1.22.6.1 -r1.22.6.1.2.1 src/sys/altq/altq_cdnr.c
cvs rdiff -u -r1.33.46.1 -r1.33.46.1.2.1 src/sys/altq/altq_subr.c
cvs rdiff -u -r1.529.2.1.2.2 -r1.529.2.1.2.3 src/sys/net/if.c
cvs rdiff -u -r1.305.2.1.2.2 -r1.305.2.1.2.3 src/sys/net/if.h

Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.

Modified files:

Index: src/sys/altq/altq_cdnr.c
diff -u src/sys/altq/altq_cdnr.c:1.22.6.1 src/sys/altq/altq_cdnr.c:1.22.6.1.2.1
--- src/sys/altq/altq_cdnr.c:1.22.6.1	Sat Nov 11 13:16:30 2023
+++ src/sys/altq/altq_cdnr.c	Wed Nov 15 02:19:00 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: altq_cdnr.c,v 1.22.6.1 2023/11/11 13:16:30 thorpej Exp $	*/
+/*	$NetBSD: altq_cdnr.c,v 1.22.6.1.2.1 2023/11/15 02:19:00 thorpej Exp $	*/
 /*	$KAME: altq_cdnr.c,v 1.15 2005/04/13 03:44:24 suz Exp $	*/
 
 /*
@@ -28,7 +28,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: altq_cdnr.c,v 1.22.6.1 2023/11/11 13:16:30 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: altq_cdnr.c,v 1.22.6.1.2.1 2023/11/15 02:19:00 thorpej Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_altq.h"
@@ -138,11 +138,16 @@ altq_cdnr_input(struct mbuf *m, int af)
 	struct tc_action	*tca;
 	struct cdnr_block	*cb;
 	struct cdnr_pktinfo	pktinfo;
+	bool			is_cnding = false;
 
 	ifp = m_get_rcvif_NOMPSAFE(m);
-	if (!ALTQ_IS_CNDTNING(&ifp->if_snd))
+	mutex_enter(ifp->if_snd.ifq_lock);
+	is_cnding = ALTQ_IS_CNDTNING(&ifp->if_snd);
+	mutex_exit(ifp->if_snd.ifq_lock);
+	if (!is_cnding) {
 		/* traffic conditioner is not enabled on this interface */
 		return (1);
+	}
 
 	top = ifp->if_snd.ifq_altq->altq_cdnr;
 
@@ -428,9 +433,11 @@ top_destroy(struct top_cdnr *top)
 {
 	struct cdnr_block *cb;
 
+	mutex_enter(top->tc_ifq->altq_ifq->ifq_lock);
 	if (ALTQ_IS_CNDTNING(top->tc_ifq))
 		ALTQ_CLEAR_CNDTNING(top->tc_ifq);
 	top->tc_ifq->altq_cdnr = NULL;
+	mutex_exit(top->tc_ifq->altq_ifq->ifq_lock);
 
 	/*
 	 * destroy all the conditioner elements belonging to this interface
@@ -448,10 +455,17 @@ top_destroy(struct top_cdnr *top)
 
 	/* if there is no active conditioner, remove the input hook */
 	if (altq_input != NULL) {
-		LIST_FOREACH(top, &tcb_list, tc_next)
+		bool is_cnding = false;
+
+		LIST_FOREACH(top, &tcb_list, tc_next) {
+			mutex_enter(top->tc_ifq->altq_ifq->ifq_lock);
 			if (ALTQ_IS_CNDTNING(top->tc_ifq))
+				is_cnding = true;
+			mutex_exit(top->tc_ifq->altq_ifq->ifq_lock);
+			if (is_cnding)
 				break;
-		if (top == NULL)
+		}
+		if (!is_cnding)
 			altq_input = NULL;
 	}
 
@@ -1220,19 +1234,30 @@ cdnrioctl(dev_t dev, ioctlcmd_t cmd, voi
 		switch (cmd) {
 
 		case CDNR_ENABLE:
+			mutex_enter(top->tc_ifq->altq_ifq->ifq_lock);
 			ALTQ_SET_CNDTNING(top->tc_ifq);
+			mutex_exit(top->tc_ifq->altq_ifq->ifq_lock);
 			if (altq_input == NULL)
 				altq_input = altq_cdnr_input;
 			break;
 
-		case CDNR_DISABLE:
+		case CDNR_DISABLE: {
+			bool is_cnding = false;
+			mutex_enter(top->tc_ifq->altq_ifq->ifq_lock);
 			ALTQ_CLEAR_CNDTNING(top->tc_ifq);
-			LIST_FOREACH(top, &tcb_list, tc_next)
+			mutex_exit(top->tc_ifq->altq_ifq->ifq_lock);
+			LIST_FOREACH(top, &tcb_list, tc_next) {
+				mutex_enter(top->tc_ifq->altq_ifq->ifq_lock);
 				if (ALTQ_IS_CNDTNING(top->tc_ifq))
+					is_cnding = true;
+				mutex_exit(top->tc_ifq->altq_ifq->ifq_lock);
+				if (is_cnding)
 					break;
-			if (top == NULL)
+			}
+			if (!is_cnding)
 				altq_input = NULL;
 			break;
+		    }
 		}
 		break;
 

Index: src/sys/altq/altq_subr.c
diff -u src/sys/altq/altq_subr.c:1.33.46.1 src/sys/altq/altq_subr.c:1.33.46.1.2.1
--- src/sys/altq/altq_subr.c:1.33.46.1	Sat Nov 11 13:16:30 2023
+++ src/sys/altq/altq_subr.c	Wed Nov 15 02:19:00 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: altq_subr.c,v 1.33.46.1 2023/11/11 13:16:30 thorpej Exp $	*/
+/*	$NetBSD: altq_subr.c,v 1.33.46.1.2.1 2023/11/15 02:19:00 thorpej Exp $	*/
 /*	$KAME: altq_subr.c,v 1.24 2005/04/13 03:44:25 suz Exp $	*/
 
 /*
@@ -28,7 +28,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: altq_subr.c,v 1.33.46.1 2023/11/11 13:16:30 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: altq_subr.c,v 1.33.46.1.2.1 2023/11/15 02:19:00 thorpej Exp $");
 
 #ifdef _KERNEL_OPT
 #include "opt_altq.h"
@@ -36,6 +36,8 @@ __KERNEL_RCSID(0, "$NetBSD: altq_subr.c,
 #include "pf.h"
 #endif
 
+#define	__IFQ_PRIVATE
+
 #include <sys/param.h>
 #include <sys/malloc.h>
 #include <sys/mbuf.h>
@@ -140,6 +142,13 @@ void
 altq_free(struct ifqueue *ifq)
 {
 	if (ifq->ifq_altq != NULL) {
+		/*
+		 * No need to pre-flight these calls; both can handle
+		 * the not-enabled / not-attached scenarios.
+		 */
+		altq_disable(ifq->ifq_altq);
+		altq_detach(ifq->ifq_altq);
+
 		ifq->ifq_altq->altq_ifp = NULL;
 		kmem_free(ifq->ifq_altq, sizeof(*ifq->ifq_altq));
 		ifq->ifq_altq = NULL;
@@ -154,7 +163,7 @@ void
 altq_set_ready(struct ifqueue *ifq)
 {
 	altq_alloc(ifq, NULL);
-	ifq->ifq_altq->altq_flags = ALTQF_READY;
+	ifq->ifq_altq->altq_flags |= ALTQF_READY;
 }
 
 /* look up the queue state by the interface name and the queueing type. */
@@ -173,14 +182,21 @@ altq_lookup(char *name, int type)
 }
 
 int
-altq_attach(struct ifaltq *ifq, int type, void *discipline,
+altq_attach(struct ifaltq *altq, int type, void *discipline,
     int (*enqueue)(struct ifaltq *, struct mbuf *),
     struct mbuf *(*dequeue)(struct ifaltq *, int),
     int (*request)(struct ifaltq *, int, void *),
     void *clfier, void *(*classify)(void *, struct mbuf *, int))
 {
-	if (!ALTQ_IS_READY(ifq))
-		return ENXIO;
+	struct ifqueue *ifq = altq->altq_ifq;
+	int error = 0;
+
+	mutex_enter(ifq->ifq_lock);
+
+	if (!ALTQ_IS_READY(ifq)) {
+		error = ENXIO;
+		goto out;
+	}
 
 #ifdef ALTQ3_COMPAT
 	/*
@@ -188,88 +204,123 @@ altq_attach(struct ifaltq *ifq, int type
 	 * check these if clfier is not NULL (which implies altq3).
 	 */
 	if (clfier != NULL) {
-		if (ALTQ_IS_ENABLED(ifq))
-			return EBUSY;
-		if (ALTQ_IS_ATTACHED(ifq))
-			return EEXIST;
+		if (ALTQ_IS_ENABLED(ifq)) {
+			error = EBUSY;
+			goto out;
+		}
+		if (ALTQ_IS_ATTACHED(ifq)) {
+			error = EEXIST;
+			goto out;
+		}
 	}
 #endif
-	ifq->altq_type     = type;
-	ifq->altq_disc     = discipline;
-	ifq->altq_enqueue  = enqueue;
-	ifq->altq_dequeue  = dequeue;
-	ifq->altq_request  = request;
-	ifq->altq_clfier   = clfier;
-	ifq->altq_classify = classify;
-	ifq->altq_flags &= (ALTQF_CANTCHANGE|ALTQF_ENABLED);
+	altq->altq_type     = type;
+	altq->altq_disc     = discipline;
+	altq->altq_enqueue  = enqueue;
+	altq->altq_dequeue  = dequeue;
+	altq->altq_request  = request;
+	altq->altq_clfier   = clfier;
+	altq->altq_classify = classify;
+	altq->altq_flags   &= (ALTQF_CANTCHANGE|ALTQF_ENABLED);
 #ifdef ALTQ3_COMPAT
 #ifdef ALTQ_KLD
 	altq_module_incref(type);
 #endif
 #endif
-	return 0;
+ out:
+	mutex_exit(ifq->ifq_lock);
+	return error;
 }
 
 int
-altq_detach(struct ifaltq *ifq)
+altq_detach(struct ifaltq *altq)
 {
-	if (!ALTQ_IS_READY(ifq))
-		return ENXIO;
-	if (ALTQ_IS_ENABLED(ifq))
-		return EBUSY;
-	if (!ALTQ_IS_ATTACHED(ifq))
-		return (0);
+	struct ifqueue *ifq = altq->altq_ifq;
+	int error = 0;
+
+	mutex_enter(ifq->ifq_lock);
+
+	if (!ALTQ_IS_READY(ifq)) {
+		error = ENXIO;
+		goto out;
+	}
+	if (ALTQ_IS_ENABLED(ifq)) {
+		error = EBUSY;
+		goto out;
+	}
+	if (!ALTQ_IS_ATTACHED(ifq)) {
+		goto out;
+	}
+
 #ifdef ALTQ3_COMPAT
 #ifdef ALTQ_KLD
 	altq_module_declref(ifq->altq_type);
 #endif
 #endif
 
-	ifq->altq_type     = ALTQT_NONE;
-	ifq->altq_disc     = NULL;
-	ifq->altq_enqueue  = NULL;
-	ifq->altq_dequeue  = NULL;
-	ifq->altq_request  = NULL;
-	ifq->altq_clfier   = NULL;
-	ifq->altq_classify = NULL;
-	ifq->altq_flags &= ALTQF_CANTCHANGE;
-	return 0;
+	altq->altq_type     = ALTQT_NONE;
+	altq->altq_disc     = NULL;
+	altq->altq_enqueue  = NULL;
+	altq->altq_dequeue  = NULL;
+	altq->altq_request  = NULL;
+	altq->altq_clfier   = NULL;
+	altq->altq_classify = NULL;
+	altq->altq_flags   &= ALTQF_CANTCHANGE;
+ out:
+	mutex_exit(ifq->ifq_lock);
+	return error;
 }
 
 int
-altq_enable(struct ifaltq *ifq)
+altq_enable(struct ifaltq *altq)
 {
-	int s;
+	struct ifqueue *ifq = altq->altq_ifq;
+	struct mbuf *m = NULL;
+	int error = 0;
 
-	if (!ALTQ_IS_READY(ifq))
-		return ENXIO;
-	if (ALTQ_IS_ENABLED(ifq))
-		return 0;
+	mutex_enter(ifq->ifq_lock);
 
-	s = splnet();
-	IFQ_PURGE(ifq->altq_ifq);
-	ASSERT(ALTQ_GET_LEN(ifq) == 0);
-	ifq->altq_flags |= ALTQF_ENABLED;
-	if (ifq->altq_clfier != NULL)
-		ifq->altq_flags |= ALTQF_CLASSIFY;
-	splx(s);
+	if (!ALTQ_IS_READY(ifq)) {
+		error = ENXIO;
+		goto out;
+	}
+	if (ALTQ_IS_ENABLED(ifq)) {
+		goto out;
+	}
 
-	return 0;
+	m = ifq_purge_locked(ifq);
+	ASSERT(ALTQ_GET_LEN(altq) == 0);
+	altq->altq_flags |= ALTQF_ENABLED;
+	if (altq->altq_clfier != NULL)
+		altq->altq_flags |= ALTQF_CLASSIFY;
+ out:
+	mutex_exit(ifq->ifq_lock);
+	if (m != NULL) {
+		ifq_purge_free(m);
+	}
+	return error;
 }
 
 int
-altq_disable(struct ifaltq *ifq)
+altq_disable(struct ifaltq *altq)
 {
-	int s;
+	struct ifqueue *ifq = altq->altq_ifq;
+	struct mbuf *m = NULL;
 
-	if (!ALTQ_IS_ENABLED(ifq))
-		return 0;
+	mutex_enter(ifq->ifq_lock);
 
-	s = splnet();
-	IFQ_PURGE(ifq->altq_ifq);
-	ASSERT(ALTQ_GET_LEN(ifq) == 0);
-	ifq->altq_flags &= ~(ALTQF_ENABLED|ALTQF_CLASSIFY);
-	splx(s);
+	if (!ALTQ_IS_ENABLED(ifq)) {
+		goto out;
+	}
+
+	m = ifq_purge_locked(ifq);
+	ASSERT(ALTQ_GET_LEN(altq) == 0);
+	altq->altq_flags &= ~(ALTQF_ENABLED|ALTQF_CLASSIFY);
+ out:
+ 	mutex_exit(ifq->ifq_lock);
+	if (m != NULL) {
+		ifq_purge_free(m);
+	}
 	return 0;
 }
 

Index: src/sys/net/if.c
diff -u src/sys/net/if.c:1.529.2.1.2.2 src/sys/net/if.c:1.529.2.1.2.3
--- src/sys/net/if.c:1.529.2.1.2.2	Wed Nov 15 02:08:33 2023
+++ src/sys/net/if.c	Wed Nov 15 02:19:00 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: if.c,v 1.529.2.1.2.2 2023/11/15 02:08:33 thorpej Exp $	*/
+/*	$NetBSD: if.c,v 1.529.2.1.2.3 2023/11/15 02:19:00 thorpej Exp $	*/
 
 /*-
  * Copyright (c) 1999, 2000, 2001, 2008, 2023 The NetBSD Foundation, Inc.
@@ -90,7 +90,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.529.2.1.2.2 2023/11/15 02:08:33 thorpej Exp $");
+__KERNEL_RCSID(0, "$NetBSD: if.c,v 1.529.2.1.2.3 2023/11/15 02:19:00 thorpej Exp $");
 
 #if defined(_KERNEL_OPT)
 #include "opt_inet.h"
@@ -101,6 +101,8 @@ __KERNEL_RCSID(0, "$NetBSD: if.c,v 1.529
 #include "opt_mrouting.h"
 #endif
 
+#define	__IFQ_PRIVATE
+
 #include <sys/param.h>
 #include <sys/mbuf.h>
 #include <sys/systm.h>
@@ -1387,10 +1389,6 @@ if_detach(struct ifnet *ifp)
 	if_down_deactivated(ifp);
 
 #ifdef ALTQ
-	if (ALTQ_IS_ENABLED(&ifp->if_snd))
-		altq_disable(ifp->if_snd.ifq_altq);
-	if (ALTQ_IS_ATTACHED(&ifp->if_snd))
-		altq_detach(ifp->if_snd.ifq_altq);
 	altq_free(&ifp->if_snd);
 #endif
 
@@ -4342,12 +4340,10 @@ ifq_purge_slow(struct ifqueue * const if
  *
  *	Purge all of the packets from the specified interface queue.
  */
-void
-ifq_purge(struct ifqueue * const ifq)
+struct mbuf *
+ifq_purge_locked(struct ifqueue * const ifq)
 {
-	struct mbuf *m, *nextm;
-
-	mutex_enter(ifq->ifq_lock);
+	struct mbuf *m;
 
 #ifdef ALTQ
 	if (__predict_false(ALTQ_IS_ENABLED(ifq)))
@@ -4366,7 +4362,13 @@ ifq_purge(struct ifqueue * const ifq)
 		ifq->ifq_staged = NULL;
 	}
 
-	mutex_exit(ifq->ifq_lock);
+	return m;
+}
+
+void
+ifq_purge_free(struct mbuf *m)
+{
+	struct mbuf *nextm;
 
 	for (; m != NULL; m = nextm) {
 		nextm = m->m_nextpkt;
@@ -4375,6 +4377,18 @@ ifq_purge(struct ifqueue * const ifq)
 	}
 }
 
+void
+ifq_purge(struct ifqueue * const ifq)
+{
+	struct mbuf *m;
+
+	mutex_enter(ifq->ifq_lock);
+	m = ifq_purge_locked(ifq);
+	mutex_exit(ifq->ifq_lock);
+
+	ifq_purge_free(m);
+}
+
 /*
  * ifq_classify_packet --
  *

Index: src/sys/net/if.h
diff -u src/sys/net/if.h:1.305.2.1.2.2 src/sys/net/if.h:1.305.2.1.2.3
--- src/sys/net/if.h:1.305.2.1.2.2	Wed Nov 15 02:08:34 2023
+++ src/sys/net/if.h	Wed Nov 15 02:19:00 2023
@@ -1,4 +1,4 @@
-/*	$NetBSD: if.h,v 1.305.2.1.2.2 2023/11/15 02:08:34 thorpej Exp $	*/
+/*	$NetBSD: if.h,v 1.305.2.1.2.3 2023/11/15 02:19:00 thorpej Exp $	*/
 
 /*-
  * Copyright (c) 1999, 2000, 2001, 2023 The NetBSD Foundation, Inc.
@@ -1270,6 +1270,11 @@ void		ifq_purge(struct ifqueue *);
 void		ifq_classify_packet(struct ifqueue *, struct mbuf *,
 				    sa_family_t);
 
+#ifdef __IFQ_PRIVATE
+struct mbuf *	ifq_purge_locked(struct ifqueue *);
+void		ifq_purge_free(struct mbuf *);
+#endif /* __IFQ_PRIVATE */
+
 int if_tunnel_check_nesting(struct ifnet *, struct mbuf *, int);
 percpu_t *if_tunnel_alloc_ro_percpu(void);
 void if_tunnel_free_ro_percpu(percpu_t *);

Reply via email to