From: mvs <mvs@openbsd.org>
Date: Thu, 3 Aug 2023 09:49:08 +0000 (+0000)
Subject: Move solock() down to sosetopt(). A part of standalone sblock() work.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=ee0786397647feeed33b08a32051be86b74b8d2c;p=openbsd

Move solock() down to sosetopt(). A part of standalone sblock() work.
This movement required because buffers related SO_SND* and SO_RCV*
socket options should be protected with sblock(). However, standalone
sblock() has different lock order with solock() and `so_snd' and
`so_rcv' buffers. At least sblock() for `so_snd' buffer will always be
taken before solock() in the sosend() path.

The (*pr_ctloutput)() call was removed from the SOL_SOCKET level 'else'
branch. Except the SO_RTABLE case where it handled in the special way,
this is null op call.

For SO_SND* and SO_RCV* cases solock() will be replaced by sblock() in
the future.

Feedback from bluhm

Tested by bluhm naddy

ok bluhm
---

diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index 9824d161c0c..af0f62164a9 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_socket.c,v 1.306 2023/07/22 14:30:39 mvs Exp $	*/
+/*	$OpenBSD: uipc_socket.c,v 1.307 2023/08/03 09:49:08 mvs Exp $	*/
 /*	$NetBSD: uipc_socket.c,v 1.21 1996/02/04 02:17:52 christos Exp $	*/
 
 /*
@@ -1789,12 +1789,12 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
 {
 	int error = 0;
 
-	soassertlocked(so);
-
 	if (level != SOL_SOCKET) {
 		if (so->so_proto->pr_ctloutput) {
+			solock(so);
 			error = (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so,
 			    level, optname, m);
+			sounlock(so);
 			return (error);
 		}
 		error = ENOPROTOOPT;
@@ -1813,9 +1813,16 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
 			    mtod(m, struct linger *)->l_linger < 0 ||
 			    mtod(m, struct linger *)->l_linger > SHRT_MAX)
 				return (EINVAL);
+
+			solock(so);
 			so->so_linger = mtod(m, struct linger *)->l_linger;
-			/* FALLTHROUGH */
+			if (*mtod(m, int *))
+				so->so_options |= optname;
+			else
+				so->so_options &= ~optname;
+			sounlock(so);
 
+			break;
 		case SO_BINDANY:
 		case SO_DEBUG:
 		case SO_KEEPALIVE:
@@ -1828,12 +1835,15 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
 		case SO_ZEROIZE:
 			if (m == NULL || m->m_len < sizeof (int))
 				return (EINVAL);
+
+			solock(so);
 			if (*mtod(m, int *))
 				so->so_options |= optname;
 			else
 				so->so_options &= ~optname;
-			break;
+			sounlock(so);
 
+			break;
 		case SO_DONTROUTE:
 			if (m == NULL || m->m_len < sizeof (int))
 				return (EINVAL);
@@ -1853,23 +1863,32 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
 			cnt = *mtod(m, int *);
 			if ((long)cnt <= 0)
 				cnt = 1;
-			switch (optname) {
 
+			solock(so);
+			switch (optname) {
 			case SO_SNDBUF:
-				if (so->so_snd.sb_state & SS_CANTSENDMORE)
-					return (EINVAL);
+				if (so->so_snd.sb_state & SS_CANTSENDMORE) {
+					error = EINVAL;
+					break;
+				}
 				if (sbcheckreserve(cnt, so->so_snd.sb_wat) ||
-				    sbreserve(so, &so->so_snd, cnt))
-					return (ENOBUFS);
+				    sbreserve(so, &so->so_snd, cnt)) {
+					error = ENOBUFS;
+					break;
+				}
 				so->so_snd.sb_wat = cnt;
 				break;
 
 			case SO_RCVBUF:
-				if (so->so_rcv.sb_state & SS_CANTRCVMORE)
-					return (EINVAL);
+				if (so->so_rcv.sb_state & SS_CANTRCVMORE) {
+					error = EINVAL;
+					break;
+				}
 				if (sbcheckreserve(cnt, so->so_rcv.sb_wat) ||
-				    sbreserve(so, &so->so_rcv, cnt))
-					return (ENOBUFS);
+				    sbreserve(so, &so->so_rcv, cnt)) {
+					error = ENOBUFS;
+					break;
+				}
 				so->so_rcv.sb_wat = cnt;
 				break;
 
@@ -1884,6 +1903,7 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
 				    so->so_rcv.sb_hiwat : cnt;
 				break;
 			}
+			sounlock(so);
 			break;
 		    }
 
@@ -1903,8 +1923,9 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
 				return (EDOM);
 			if (nsecs == 0)
 				nsecs = INFSLP;
-			switch (optname) {
 
+			solock(so);
+			switch (optname) {
 			case SO_SNDTIMEO:
 				so->so_snd.sb_timeo_nsecs = nsecs;
 				break;
@@ -1912,6 +1933,7 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
 				so->so_rcv.sb_timeo_nsecs = nsecs;
 				break;
 			}
+			sounlock(so);
 			break;
 		    }
 
@@ -1923,19 +1945,20 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
 				    so->so_proto->pr_domain;
 
 				level = dom->dom_protosw->pr_protocol;
+				solock(so);
 				error = (*so->so_proto->pr_ctloutput)
 				    (PRCO_SETOPT, so, level, optname, m);
-				return (error);
-			}
-			error = ENOPROTOOPT;
+				sounlock(so);
+			} else
+				error = ENOPROTOOPT;
 			break;
-
 #ifdef SOCKET_SPLICE
 		case SO_SPLICE:
+			solock(so);
 			if (m == NULL) {
 				error = sosplice(so, -1, 0, NULL);
 			} else if (m->m_len < sizeof(int)) {
-				return (EINVAL);
+				error = EINVAL;
 			} else if (m->m_len < sizeof(struct splice)) {
 				error = sosplice(so, *mtod(m, int *), 0, NULL);
 			} else {
@@ -1944,6 +1967,7 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
 				    mtod(m, struct splice *)->sp_max,
 				   &mtod(m, struct splice *)->sp_idle);
 			}
+			sounlock(so);
 			break;
 #endif /* SOCKET_SPLICE */
 
@@ -1951,10 +1975,6 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m)
 			error = ENOPROTOOPT;
 			break;
 		}
-		if (error == 0 && so->so_proto->pr_ctloutput) {
-			(*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so,
-			    level, optname, m);
-		}
 	}
 
 	return (error);
diff --git a/sys/kern/uipc_syscalls.c b/sys/kern/uipc_syscalls.c
index 6b8bfc84bac..7e6cb9b2731 100644
--- a/sys/kern/uipc_syscalls.c
+++ b/sys/kern/uipc_syscalls.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_syscalls.c,v 1.212 2023/02/10 14:34:17 visa Exp $	*/
+/*	$OpenBSD: uipc_syscalls.c,v 1.213 2023/08/03 09:49:08 mvs Exp $	*/
 /*	$NetBSD: uipc_syscalls.c,v 1.19 1996/02/09 19:00:48 christos Exp $	*/
 
 /*
@@ -1232,9 +1232,7 @@ sys_setsockopt(struct proc *p, void *v, register_t *retval)
 		m->m_len = SCARG(uap, valsize);
 	}
 	so = fp->f_data;
-	solock(so);
 	error = sosetopt(so, SCARG(uap, level), SCARG(uap, name), m);
-	sounlock(so);
 bad:
 	m_freem(m);
 	FRELE(fp, p);
diff --git a/sys/net/bfd.c b/sys/net/bfd.c
index 4b105587276..9592c1b0aea 100644
--- a/sys/net/bfd.c
+++ b/sys/net/bfd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: bfd.c,v 1.79 2023/07/12 16:10:45 mvs Exp $	*/
+/*	$OpenBSD: bfd.c,v 1.80 2023/08/03 09:49:08 mvs Exp $	*/
 
 /*
  * Copyright (c) 2016-2018 Peter Hessler <phessler@openbsd.org>
@@ -452,9 +452,7 @@ bfd_listener(struct bfd_config *bfd, unsigned int port)
 	mopt->m_len = sizeof(int);
 	ip = mtod(mopt, int *);
 	*ip = MAXTTL;
-	solock(so);
 	error = sosetopt(so, IPPROTO_IP, IP_MINTTL, mopt);
-	sounlock(so);
 	m_freem(mopt);
 	if (error) {
 		printf("%s: sosetopt error %d\n",
@@ -531,9 +529,7 @@ bfd_sender(struct bfd_config *bfd, unsigned int port)
 	mopt->m_len = sizeof(int);
 	ip = mtod(mopt, int *);
 	*ip = IP_PORTRANGE_HIGH;
-	solock(so);
 	error = sosetopt(so, IPPROTO_IP, IP_PORTRANGE, mopt);
-	sounlock(so);
 	m_freem(mopt);
 	if (error) {
 		printf("%s: sosetopt error %d\n",
@@ -545,9 +541,7 @@ bfd_sender(struct bfd_config *bfd, unsigned int port)
 	mopt->m_len = sizeof(int);
 	ip = mtod(mopt, int *);
 	*ip = MAXTTL;
-	solock(so);
 	error = sosetopt(so, IPPROTO_IP, IP_TTL, mopt);
-	sounlock(so);
 	m_freem(mopt);
 	if (error) {
 		printf("%s: sosetopt error %d\n",
@@ -559,9 +553,7 @@ bfd_sender(struct bfd_config *bfd, unsigned int port)
 	mopt->m_len = sizeof(int);
 	ip = mtod(mopt, int *);
 	*ip = IPTOS_PREC_INTERNETCONTROL;
-	solock(so);
 	error = sosetopt(so, IPPROTO_IP, IP_TOS, mopt);
-	sounlock(so);
 	m_freem(mopt);
 	if (error) {
 		printf("%s: sosetopt error %d\n",
diff --git a/sys/net/if_vxlan.c b/sys/net/if_vxlan.c
index 937f36ba564..37311eb90b2 100644
--- a/sys/net/if_vxlan.c
+++ b/sys/net/if_vxlan.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_vxlan.c,v 1.92 2023/04/13 02:19:05 jsg Exp $ */
+/*	$OpenBSD: if_vxlan.c,v 1.93 2023/08/03 09:49:08 mvs Exp $ */
 
 /*
  * Copyright (c) 2021 David Gwynne <dlg@openbsd.org>
@@ -934,9 +934,9 @@ vxlan_tep_add_addr(struct vxlan_softc *sc, const union vxlan_addr *addr,
 		goto free;
 
 	solock(so);
-
 	sotoinpcb(so)->inp_upcall = vxlan_input;
 	sotoinpcb(so)->inp_upcall_arg = vt;
+	sounlock(so);
 
 	m_inithdr(&m);
 	m.m_len = sizeof(vt->vt_rdomain);
@@ -973,12 +973,12 @@ vxlan_tep_add_addr(struct vxlan_softc *sc, const union vxlan_addr *addr,
 		unhandled_af(vt->vt_af);
 	}
 
+	solock(so);
 	error = sobind(so, &m, curproc);
+	sounlock(so);
 	if (error != 0)
 		goto close;
 
-	sounlock(so);
-
 	rw_assert_wrlock(&vxlan_lock);
 	TAILQ_INSERT_TAIL(&vxlan_teps, vt, vt_entry);
 
@@ -987,7 +987,6 @@ vxlan_tep_add_addr(struct vxlan_softc *sc, const union vxlan_addr *addr,
 	return (0);
 
 close:
-	sounlock(so);
 	soclose(so, MSG_DONTWAIT);
 free:
 	free(vt, M_DEVBUF, sizeof(*vt));
diff --git a/sys/net/if_wg.c b/sys/net/if_wg.c
index 951cb6f1df0..593bfc6a6db 100644
--- a/sys/net/if_wg.c
+++ b/sys/net/if_wg.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: if_wg.c,v 1.28 2023/06/01 18:57:53 kn Exp $ */
+/*	$OpenBSD: if_wg.c,v 1.29 2023/08/03 09:49:08 mvs Exp $ */
 
 /*
  * Copyright (C) 2015-2020 Jason A. Donenfeld <Jason@zx2c4.com>. All Rights Reserved.
@@ -720,14 +720,16 @@ wg_socket_open(struct socket **so, int af, in_port_t *port,
 	solock(*so);
 	sotoinpcb(*so)->inp_upcall = wg_input;
 	sotoinpcb(*so)->inp_upcall_arg = upcall_arg;
+	sounlock(*so);
 
 	if ((ret = sosetopt(*so, SOL_SOCKET, SO_RTABLE, &mrtable)) == 0) {
+		solock(*so);
 		if ((ret = sobind(*so, &mhostnam, curproc)) == 0) {
 			*port = sotoinpcb(*so)->inp_lport;
 			*rtable = sotoinpcb(*so)->inp_rtableid;
 		}
+		sounlock(*so);
 	}
-	sounlock(*so);
 
 	if (ret != 0)
 		wg_socket_close(so);
diff --git a/sys/nfs/krpc_subr.c b/sys/nfs/krpc_subr.c
index eb35b5f6c1d..fec8ae712e8 100644
--- a/sys/nfs/krpc_subr.c
+++ b/sys/nfs/krpc_subr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: krpc_subr.c,v 1.37 2022/06/06 14:45:41 claudio Exp $	*/
+/*	$OpenBSD: krpc_subr.c,v 1.38 2023/08/03 09:49:09 mvs Exp $	*/
 /*	$NetBSD: krpc_subr.c,v 1.12.4.1 1996/06/07 00:52:26 cgd Exp $	*/
 
 /*
@@ -239,9 +239,7 @@ krpc_call(struct sockaddr_in *sa, u_int prog, u_int vers, u_int func,
 	tv.tv_usec = 0;
 	memcpy(mtod(m, struct timeval *), &tv, sizeof tv);
 	m->m_len = sizeof(tv);
-	solock(so);
 	error = sosetopt(so, SOL_SOCKET, SO_RCVTIMEO, m);
-	sounlock(so);
 	m_freem(m);
 	if (error)
 		goto out;
@@ -255,9 +253,7 @@ krpc_call(struct sockaddr_in *sa, u_int prog, u_int vers, u_int func,
 		on = mtod(m, int32_t *);
 		m->m_len = sizeof(*on);
 		*on = 1;
-		solock(so);
 		error = sosetopt(so, SOL_SOCKET, SO_BROADCAST, m);
-		sounlock(so);
 		m_freem(m);
 		if (error)
 			goto out;
@@ -272,9 +268,7 @@ krpc_call(struct sockaddr_in *sa, u_int prog, u_int vers, u_int func,
 	mopt->m_len = sizeof(int);
 	ip = mtod(mopt, int *);
 	*ip = IP_PORTRANGE_LOW;
-	solock(so);
 	error = sosetopt(so, IPPROTO_IP, IP_PORTRANGE, mopt);
-	sounlock(so);
 	m_freem(mopt);
 	if (error)
 		goto out;
@@ -299,9 +293,7 @@ krpc_call(struct sockaddr_in *sa, u_int prog, u_int vers, u_int func,
 	mopt->m_len = sizeof(int);
 	ip = mtod(mopt, int *);
 	*ip = IP_PORTRANGE_DEFAULT;
-	solock(so);
 	error = sosetopt(so, IPPROTO_IP, IP_PORTRANGE, mopt);
-	sounlock(so);
 	m_freem(mopt);
 	if (error)
 		goto out;
diff --git a/sys/nfs/nfs_socket.c b/sys/nfs/nfs_socket.c
index 5488a501040..80dfaa699c9 100644
--- a/sys/nfs/nfs_socket.c
+++ b/sys/nfs/nfs_socket.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nfs_socket.c,v 1.143 2022/08/13 21:01:46 mvs Exp $	*/
+/*	$OpenBSD: nfs_socket.c,v 1.144 2023/08/03 09:49:09 mvs Exp $	*/
 /*	$NetBSD: nfs_socket.c,v 1.27 1996/04/15 20:20:00 thorpej Exp $	*/
 
 /*
@@ -258,7 +258,6 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
 		MGET(nam, M_WAIT, MT_SONAME);
 
 	so = nmp->nm_so;
-	solock(so);
 	nmp->nm_soflags = so->so_proto->pr_flags;
 
 	/*
@@ -282,7 +281,9 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
 		sin->sin_family = AF_INET;
 		sin->sin_addr.s_addr = INADDR_ANY;
 		sin->sin_port = htons(0);
+		solock(so);
 		error = sobind(so, nam, &proc0);
+		sounlock(so);
 		if (error)
 			goto bad;
 
@@ -294,6 +295,7 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
 			goto bad;
 	}
 
+	solock(so);
 	/*
 	 * Protocols that do not require connections may be optionally left
 	 * unconnected for servers that reply from a port other than NFS_PORT.
@@ -301,12 +303,12 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
 	if (nmp->nm_flag & NFSMNT_NOCONN) {
 		if (nmp->nm_soflags & PR_CONNREQUIRED) {
 			error = ENOTCONN;
-			goto bad;
+			goto bad_locked;
 		}
 	} else {
 		error = soconnect(so, nmp->nm_nam);
 		if (error)
-			goto bad;
+			goto bad_locked;
 
 		/*
 		 * Wait for the connection to complete. Cribbed from the
@@ -320,13 +322,13 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
 			    so->so_error == 0 && rep &&
 			    (error = nfs_sigintr(nmp, rep, rep->r_procp)) != 0){
 				so->so_state &= ~SS_ISCONNECTING;
-				goto bad;
+				goto bad_locked;
 			}
 		}
 		if (so->so_error) {
 			error = so->so_error;
 			so->so_error = 0;
-			goto bad;
+			goto bad_locked;
 		}
 	}
 	/*
@@ -338,6 +340,7 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
 		so->so_snd.sb_timeo_nsecs = SEC_TO_NSEC(5);
 	else
 		so->so_snd.sb_timeo_nsecs = INFSLP;
+	sounlock(so);
 	if (nmp->nm_sotype == SOCK_DGRAM) {
 		sndreserve = nmp->nm_wsize + NFS_MAXPKTHDR;
 		rcvreserve = (max(nmp->nm_rsize, nmp->nm_readdirsize) +
@@ -360,9 +363,10 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
 	} else {
 		panic("%s: nm_sotype %d", __func__, nmp->nm_sotype);
 	}
+	solock(so);
 	error = soreserve(so, sndreserve, rcvreserve);
 	if (error)
-		goto bad;
+		goto bad_locked;
 	so->so_rcv.sb_flags |= SB_NOINTR;
 	so->so_snd.sb_flags |= SB_NOINTR;
 	sounlock(so);
@@ -377,8 +381,9 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
 	nmp->nm_timeouts = 0;
 	return (0);
 
-bad:
+bad_locked:
 	sounlock(so);
+bad:
 
 	m_freem(mopt);
 	m_freem(nam);
diff --git a/sys/nfs/nfs_syscalls.c b/sys/nfs/nfs_syscalls.c
index b9eecb0430b..45ff763af20 100644
--- a/sys/nfs/nfs_syscalls.c
+++ b/sys/nfs/nfs_syscalls.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nfs_syscalls.c,v 1.118 2022/06/06 14:45:41 claudio Exp $	*/
+/*	$OpenBSD: nfs_syscalls.c,v 1.119 2023/08/03 09:49:09 mvs Exp $	*/
 /*	$NetBSD: nfs_syscalls.c,v 1.19 1996/02/18 11:53:52 fvdl Exp $	*/
 
 /*
@@ -249,8 +249,8 @@ nfssvc_addsock(struct file *fp, struct mbuf *mynam)
 		siz = NFS_MAXPACKET;
 	solock(so);
 	error = soreserve(so, siz, siz); 
+	sounlock(so);
 	if (error) {
-		sounlock(so);
 		m_freem(mynam);
 		return (error);
 	}
@@ -275,6 +275,7 @@ nfssvc_addsock(struct file *fp, struct mbuf *mynam)
 		sosetopt(so, IPPROTO_TCP, TCP_NODELAY, m);
 		m_freem(m);
 	}
+	solock(so);
 	so->so_rcv.sb_flags &= ~SB_NOINTR;
 	so->so_rcv.sb_timeo_nsecs = INFSLP;
 	so->so_snd.sb_flags &= ~SB_NOINTR;