The mbuf_queue API allows read access to integer variables which

another CPU may change simultaneously.  To prevent miss optimisation
by the compiler, they need the READ_ONCE() macro.  Otherwise there
could be two read operations with inconsistent values.  Writing to
integer in mq_set_maxlen() needs mutex protection.  Otherwise the
value could change within critical sections.  Again the compiler
could optimize to multiple read operations within the critical
section.  With inconsistent values, the behavior is undefined.
OK dlg@

diff --git a/sys/kern/uipc_mbuf.c b/sys/kern/uipc_mbuf.c
index ffafd22..6f0ea19 100644 (file)
--- a/sys/kern/uipc_mbuf.c
+++ b/sys/kern/uipc_mbuf.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: uipc_mbuf.c,v 1.284 2022/08/14 01:58:28 jsg Exp $     */
+/*     $OpenBSD: uipc_mbuf.c,v 1.285 2023/05/05 01:19:51 bluhm Exp $   */
 /*     $NetBSD: uipc_mbuf.c,v 1.15.4.1 1996/06/13 17:11:44 cgd Exp $   */
 
 /*
@@ -1776,6 +1776,14 @@ mq_hdatalen(struct mbuf_queue *mq)
        return (hdatalen);
 }
 
+void
+mq_set_maxlen(struct mbuf_queue *mq, u_int maxlen)
+{
+       mtx_enter(&mq->mq_mtx);
+       mq->mq_maxlen = maxlen;
+       mtx_leave(&mq->mq_mtx);
+}
+
 int
 sysctl_mq(int *name, u_int namelen, void *oldp, size_t *oldlenp,
     void *newp, size_t newlen, struct mbuf_queue *mq)
@@ -1793,11 +1801,8 @@ sysctl_mq(int *name, u_int namelen, void *oldp, size_t *oldlenp,
        case IFQCTL_MAXLEN:
                maxlen = mq->mq_maxlen;
                error = sysctl_int(oldp, oldlenp, newp, newlen, &maxlen);
-               if (!error && maxlen != mq->mq_maxlen) {
-                       mtx_enter(&mq->mq_mtx);
-                       mq->mq_maxlen = maxlen;
-                       mtx_leave(&mq->mq_mtx);
-               }
+               if (!error && maxlen != mq->mq_maxlen)
+                       mq_set_maxlen(mq, maxlen);
                return (error);
        case IFQCTL_DROPS:
                return (sysctl_rdint(oldp, oldlenp, newp, mq_drops(mq)));

diff --git a/sys/sys/mbuf.h b/sys/sys/mbuf.h
index f8eae7a..65d5c5b 100644 (file)
--- a/sys/sys/mbuf.h
+++ b/sys/sys/mbuf.h
@@ -1,4 +1,4 @@
-/*     $OpenBSD: mbuf.h,v 1.255 2022/08/15 16:15:37 bluhm Exp $        */
+/*     $OpenBSD: mbuf.h,v 1.256 2023/05/05 01:19:51 bluhm Exp $        */
 /*     $NetBSD: mbuf.h,v 1.19 1996/02/09 18:25:14 christos Exp $       */
 
 /*
@@ -526,6 +526,8 @@ unsigned int                ml_hdatalen(struct mbuf_list *);
  * mbuf queues
  */
 
+#include <sys/atomic.h>
+
 #define MBUF_QUEUE_INITIALIZER(_maxlen, _ipl) \
     { MUTEX_INITIALIZER(_ipl), MBUF_LIST_INITIALIZER(), (_maxlen), 0 }
 
@@ -538,12 +540,12 @@ void                      mq_delist(struct mbuf_queue *, struct mbuf_list *);
 struct mbuf *          mq_dechain(struct mbuf_queue *);
 unsigned int           mq_purge(struct mbuf_queue *);
 unsigned int           mq_hdatalen(struct mbuf_queue *);
+void                   mq_set_maxlen(struct mbuf_queue *, u_int);
 
-#define        mq_len(_mq)             ml_len(&(_mq)->mq_list)
-#define        mq_empty(_mq)           ml_empty(&(_mq)->mq_list)
-#define        mq_full(_mq)            (mq_len((_mq)) >= (_mq)->mq_maxlen)
-#define        mq_drops(_mq)           ((_mq)->mq_drops)
-#define        mq_set_maxlen(_mq, _l)  ((_mq)->mq_maxlen = (_l))
+#define mq_len(_mq)            READ_ONCE((_mq)->mq_list.ml_len)
+#define mq_empty(_mq)          (mq_len(_mq) == 0)
+#define mq_full(_mq)           (mq_len((_mq)) >= READ_ONCE((_mq)->mq_maxlen))
+#define mq_drops(_mq)          READ_ONCE((_mq)->mq_drops)
 
 #endif /* _KERNEL */
 #endif /* _SYS_MBUF_H_ */