From: Alexander Bluhm <bluhm@openbsd.org>
Subject: Re: MP-safe sysctl_mq()
To: Vitaliy Makkoveev <mvs@openbsd.org>
Cc: tech@openbsd.org
Date: Thu, 17 Jul 2025 11:27:03 +0200

On Wed, Jul 16, 2025 at 02:02:00AM +0300, Vitaliy Makkoveev wrote:
> sysctl_mq() is wrapped by sysctl_niq() macro and called by ip_sysctl().
> 
> Actually it is mp-safe, but I want to use "oldval != newval" style to
> avoid `mq_mtx' acquisition while `mq_maxlen' was not changed. It has
> type of unsigned int, but protected with `mq_mtx' mutex(9), so I think
> to use READ_ONCE() to ensure that `mq_maxlen' actually loaded to
> `oldval'. mq_set_maxlen() takes `mq_mtx' mutex(9) within.

OK bluhm@

> Index: sys/kern/uipc_mbuf.c
> ===================================================================
> RCS file: /cvs/src/sys/kern/uipc_mbuf.c,v
> retrieving revision 1.300
> diff -u -p -r1.300 uipc_mbuf.c
> --- sys/kern/uipc_mbuf.c	25 Jun 2025 20:26:32 -0000	1.300
> +++ sys/kern/uipc_mbuf.c	15 Jul 2025 22:58:03 -0000
> @@ -1839,7 +1839,7 @@ int
>  sysctl_mq(int *name, u_int namelen, void *oldp, size_t *oldlenp,
>      void *newp, size_t newlen, struct mbuf_queue *mq)
>  {
> -	unsigned int maxlen;
> +	unsigned int oldval, newval;
>  	int error;
>  
>  	/* All sysctl names at this level are terminal. */
> @@ -1850,10 +1850,10 @@ sysctl_mq(int *name, u_int namelen, void
>  	case IFQCTL_LEN:
>  		return (sysctl_rdint(oldp, oldlenp, newp, mq_len(mq)));
>  	case IFQCTL_MAXLEN:
> -		maxlen = mq->mq_maxlen;
> -		error = sysctl_int(oldp, oldlenp, newp, newlen, &maxlen);
> -		if (error == 0)
> -			mq_set_maxlen(mq, maxlen);
> +		oldval = newval = READ_ONCE(mq->mq_maxlen);
> +		error = sysctl_int(oldp, oldlenp, newp, newlen, &newval);
> +		if (error == 0 && oldval != newval)
> +			mq_set_maxlen(mq, newval);
>  		return (error);
>  	case IFQCTL_DROPS:
>  		return (sysctl_rdint(oldp, oldlenp, newp, mq_drops(mq)));