From: bluhm <bluhm@openbsd.org>
Date: Fri, 5 May 2023 01:19:51 +0000 (+0000)
Subject: The mbuf_queue API allows read access to integer variables which
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=16d357f8d50d8e9d96c06fec2d02ae86da153511;p=openbsd

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 ffafd228522..6f0ea19b014 100644
--- 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 f8eae7a4e50..65d5c5b5897 100644
--- 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_ */