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