Hi,
The mbuf_queue API allows read access to integer variables which
another CPU may change simultaneously. To prevent miss-optimisations
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 during critical sections. Again
the compiler could optimize to multiple read operations within the
critical section. With inconsistent values, the behavior is
undefined.
The header file diff survived a make release.
ok?
bluhm
Index: kern/uipc_mbuf.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_mbuf.c,v
retrieving revision 1.284
diff -u -p -r1.284 uipc_mbuf.c
--- kern/uipc_mbuf.c 14 Aug 2022 01:58:28 -0000 1.284
+++ kern/uipc_mbuf.c 4 May 2023 07:24:13 -0000
@@ -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
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)));
Index: sys/mbuf.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/sys/mbuf.h,v
retrieving revision 1.255
diff -u -p -r1.255 mbuf.h
--- sys/mbuf.h 15 Aug 2022 16:15:37 -0000 1.255
+++ sys/mbuf.h 4 May 2023 07:26:00 -0000
@@ -526,6 +526,8 @@ unsigned int ml_hdatalen(struct mbuf_li
* 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 *,
st
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_ */
