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 *);