Move the socket lock "above" sosetopt(), sogetopt() and sosplice().

Protect the fields modifieds by sosetopt() and simplify the dance
with the stars.

ok bluhm@

5 files changed, 48 insertions, 47 deletions

diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index 64a65603f25..10f3760d469 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_socket.c,v 1.198 2017/07/27 12:05:36 mpi Exp $	*/
+/*	$OpenBSD: uipc_socket.c,v 1.199 2017/08/09 14:22:58 mpi Exp $	*/
 /*	$NetBSD: uipc_socket.c,v 1.21 1996/02/04 02:17:52 christos Exp $	*/
 
 /*
@@ -1073,7 +1073,9 @@ sosplice(struct socket *so, int fd, off_t max, struct timeval *tv)
 	struct file	*fp;
 	struct socket	*sosp;
 	struct sosplice	*sp;
-	int		 s, error = 0;
+	int		 error = 0;
+
+	soassertlocked(so);
 
 	if (sosplice_taskq == NULL)
 		sosplice_taskq = taskq_create("sosplice", 1, IPL_SOFTNET, 0);
@@ -1097,17 +1099,14 @@ sosplice(struct socket *so, int fd, off_t max, struct timeval *tv)
 
 	/* If no fd is given, unsplice by removing existing link. */
 	if (fd < 0) {
-		s = solock(so);
 		/* Lock receive buffer. */
 		if ((error = sblock(so, &so->so_rcv,
 		    (so->so_state & SS_NBIO) ? M_NOWAIT : M_WAITOK)) != 0) {
-			sounlock(s);
 			return (error);
 		}
 		if (so->so_sp->ssp_socket)
 			sounsplice(so, so->so_sp->ssp_socket, 1);
 		sbunlock(&so->so_rcv);
-		sounlock(s);
 		return (0);
 	}
 
@@ -1129,17 +1128,14 @@ sosplice(struct socket *so, int fd, off_t max, struct timeval *tv)
 			pool_put(&sosplice_pool, sp);
 	}
 
-	s = solock(so);
 	/* Lock both receive and send buffer. */
 	if ((error = sblock(so, &so->so_rcv,
 	    (so->so_state & SS_NBIO) ? M_NOWAIT : M_WAITOK)) != 0) {
-		sounlock(s);
 		FRELE(fp, curproc);
 		return (error);
 	}
 	if ((error = sblock(so, &sosp->so_snd, M_WAITOK)) != 0) {
 		sbunlock(&so->so_rcv);
-		sounlock(s);
 		FRELE(fp, curproc);
 		return (error);
 	}
@@ -1185,7 +1181,6 @@ sosplice(struct socket *so, int fd, off_t max, struct timeval *tv)
  release:
 	sbunlock(&sosp->so_snd);
 	sbunlock(&so->so_rcv);
-	sounlock(s);
 	FRELE(fp, curproc);
 	return (error);
 }
@@ -1565,15 +1560,15 @@ sowwakeup(struct socket *so)
 int
 sosetopt(struct socket *so, int level, int optname, struct mbuf *m0)
 {
-	int s, error = 0;
+	int error = 0;
 	struct mbuf *m = m0;
 
+	soassertlocked(so);
+
 	if (level != SOL_SOCKET) {
 		if (so->so_proto && so->so_proto->pr_ctloutput) {
-			s = solock(so);
 			error = (*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so,
 			    level, optname, m0);
-			sounlock(s);
 			return (error);
 		}
 		error = ENOPROTOOPT;
@@ -1647,14 +1642,11 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m0)
 					error = EINVAL;
 					goto bad;
 				}
-				s = solock(so);
 				if (sbcheckreserve(cnt, so->so_snd.sb_wat) ||
 				    sbreserve(so, &so->so_snd, cnt)) {
-				    	sounlock(s);
 					error = ENOBUFS;
 					goto bad;
 				}
-				sounlock(s);
 				so->so_snd.sb_wat = cnt;
 				break;
 
@@ -1663,14 +1655,11 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m0)
 					error = EINVAL;
 					goto bad;
 				}
-				s = solock(so);
 				if (sbcheckreserve(cnt, so->so_rcv.sb_wat) ||
 				    sbreserve(so, &so->so_rcv, cnt)) {
-					sounlock(s);
 					error = ENOBUFS;
 					goto bad;
 				}
-				sounlock(s);
 				so->so_rcv.sb_wat = cnt;
 				break;
 
@@ -1724,10 +1713,8 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m0)
 				struct domain *dom = so->so_proto->pr_domain;
 
 				level = dom->dom_protosw->pr_protocol;
-				s = solock(so);
 				error = (*so->so_proto->pr_ctloutput)
 				    (PRCO_SETOPT, so, level, optname, m0);
-				sounlock(s);
 				return (error);
 			}
 			error = ENOPROTOOPT;
@@ -1756,10 +1743,8 @@ sosetopt(struct socket *so, int level, int optname, struct mbuf *m0)
 			break;
 		}
 		if (error == 0 && so->so_proto && so->so_proto->pr_ctloutput) {
-			s = solock(so);
 			(*so->so_proto->pr_ctloutput)(PRCO_SETOPT, so,
 			    level, optname, m0);
-			sounlock(s);
 			m = NULL;	/* freed by protocol */
 		}
 	}
@@ -1772,18 +1757,18 @@ bad:
 int
 sogetopt(struct socket *so, int level, int optname, struct mbuf **mp)
 {
-	int s, error = 0;
+	int error = 0;
 	struct mbuf *m;
 
+	soassertlocked(so);
+
 	if (level != SOL_SOCKET) {
 		if (so->so_proto && so->so_proto->pr_ctloutput) {
 			m = m_get(M_WAIT, MT_SOOPTS);
 			m->m_len = 0;
 
-			s = solock(so);
 			error = (*so->so_proto->pr_ctloutput)(PRCO_GETOPT, so,
 			    level, optname, m);
-			sounlock(s);
 			if (error) {
 				m_free(m);
 				return (error);
@@ -1869,10 +1854,8 @@ sogetopt(struct socket *so, int level, int optname, struct mbuf **mp)
 				struct domain *dom = so->so_proto->pr_domain;
 
 				level = dom->dom_protosw->pr_protocol;
-				s = solock(so);
 				error = (*so->so_proto->pr_ctloutput)
 				    (PRCO_GETOPT, so, level, optname, m);
-				sounlock(s);
 				if (error) {
 					(void)m_free(m);
 					return (error);
@@ -1886,13 +1869,10 @@ sogetopt(struct socket *so, int level, int optname, struct mbuf **mp)
 		case SO_SPLICE:
 		    {
 			off_t len;
-			int s;
 
-			s = solock(so);
 			m->m_len = sizeof(off_t);
 			len = so->so_sp ? so->so_sp->ssp_len : 0;
 			memcpy(mtod(m, off_t *), &len, sizeof(off_t));
-			sounlock(s);
 			break;
 		    }
 #endif /* SOCKET_SPLICE */
diff --git a/sys/kern/uipc_syscalls.c b/sys/kern/uipc_syscalls.c
index ac47bef508d..21fb463f8c9 100644
--- a/sys/kern/uipc_syscalls.c
+++ b/sys/kern/uipc_syscalls.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_syscalls.c,v 1.156 2017/07/24 15:07:39 mpi Exp $	*/
+/*	$OpenBSD: uipc_syscalls.c,v 1.157 2017/08/09 14:22:58 mpi Exp $	*/
 /*	$NetBSD: uipc_syscalls.c,v 1.19 1996/02/09 19:00:48 christos Exp $	*/
 
 /*
@@ -940,7 +940,8 @@ sys_setsockopt(struct proc *p, void *v, register_t *retval)
 	} */ *uap = v;
 	struct file *fp;
 	struct mbuf *m = NULL;
-	int error;
+	struct socket *so;
+	int s, error;
 
 
 	if ((error = getsock(p, SCARG(uap, s), &fp)) != 0)
@@ -972,7 +973,10 @@ sys_setsockopt(struct proc *p, void *v, register_t *retval)
 		}
 		m->m_len = SCARG(uap, valsize);
 	}
-	error = sosetopt(fp->f_data, SCARG(uap, level), SCARG(uap, name), m);
+	so = fp->f_data;
+	s = solock(so);
+	error = sosetopt(so, SCARG(uap, level), SCARG(uap, name), m);
+	sounlock(s);
 	m = NULL;
 bad:
 	m_freem(m);
@@ -993,7 +997,8 @@ sys_getsockopt(struct proc *p, void *v, register_t *retval)
 	struct file *fp;
 	struct mbuf *m = NULL;
 	socklen_t valsize;
-	int error;
+	struct socket *so;
+	int s, error;
 
 	if ((error = getsock(p, SCARG(uap, s), &fp)) != 0)
 		return (error);
@@ -1007,9 +1012,11 @@ sys_getsockopt(struct proc *p, void *v, register_t *retval)
 			goto out;
 	} else
 		valsize = 0;
-	if ((error = sogetopt(fp->f_data, SCARG(uap, level),
-	    SCARG(uap, name), &m)) == 0 && SCARG(uap, val) && valsize &&
-	    m != NULL) {
+	so = fp->f_data;
+	s = solock(so);
+	error = sogetopt(so, SCARG(uap, level), SCARG(uap, name), &m);
+	sounlock(s);
+	if (error == 0 && SCARG(uap, val) && valsize && m != NULL) {
 		if (valsize > m->m_len)
 			valsize = m->m_len;
 		error = copyout(mtod(m, caddr_t), SCARG(uap, val), valsize);
diff --git a/sys/nfs/krpc_subr.c b/sys/nfs/krpc_subr.c
index eddad29015a..780a62a1e99 100644
--- a/sys/nfs/krpc_subr.c
+++ b/sys/nfs/krpc_subr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: krpc_subr.c,v 1.29 2015/08/24 14:00:29 bluhm Exp $	*/
+/*	$OpenBSD: krpc_subr.c,v 1.30 2017/08/09 14:22:58 mpi Exp $	*/
 /*	$NetBSD: krpc_subr.c,v 1.12.4.1 1996/06/07 00:52:26 cgd Exp $	*/
 
 /*
@@ -211,7 +211,7 @@ krpc_call(struct sockaddr_in *sa, u_int prog, u_int vers, u_int func,
 	struct rpc_call *call;
 	struct rpc_reply *reply;
 	struct uio auio;
-	int error, rcvflg, timo, secs, len;
+	int s, error, rcvflg, timo, secs, len;
 	static u_int32_t xid = 0;
 	char addr[INET_ADDRSTRLEN];
 	int *ip;
@@ -239,7 +239,10 @@ 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);
-	if ((error = sosetopt(so, SOL_SOCKET, SO_RCVTIMEO, m)))
+	s = solock(so);
+	error = sosetopt(so, SOL_SOCKET, SO_RCVTIMEO, m);
+	sounlock(s);
+	if (error)
 		goto out;
 
 	/*
@@ -251,7 +254,10 @@ 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;
-		if ((error = sosetopt(so, SOL_SOCKET, SO_BROADCAST, m)))
+		s = solock(so);
+		error = sosetopt(so, SOL_SOCKET, SO_BROADCAST, m);
+		sounlock(s);
+		if (error)
 			goto out;
 	}
 
@@ -264,7 +270,9 @@ 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;
+	s = solock(so);
 	error = sosetopt(so, IPPROTO_IP, IP_PORTRANGE, mopt);
+	sounlock(s);
 	if (error)
 		goto out;
 
@@ -286,7 +294,9 @@ 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;
+	s = solock(so);
 	error = sosetopt(so, IPPROTO_IP, IP_PORTRANGE, mopt);
+	sounlock(s);
 	if (error)
 		goto out;
 
diff --git a/sys/nfs/nfs_socket.c b/sys/nfs/nfs_socket.c
index ae2ebb0fd35..d8ec563d351 100644
--- a/sys/nfs/nfs_socket.c
+++ b/sys/nfs/nfs_socket.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nfs_socket.c,v 1.120 2017/07/24 15:07:39 mpi Exp $	*/
+/*	$OpenBSD: nfs_socket.c,v 1.121 2017/08/09 14:22:58 mpi Exp $	*/
 /*	$NetBSD: nfs_socket.c,v 1.27 1996/04/15 20:20:00 thorpej Exp $	*/
 
 /*
@@ -262,7 +262,9 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
 		mopt->m_len = sizeof(int);
 		ip = mtod(mopt, int *);
 		*ip = IP_PORTRANGE_LOW;
+		s = solock(so);
 		error = sosetopt(so, IPPROTO_IP, IP_PORTRANGE, mopt);
+		sounlock(s);
 		if (error)
 			goto bad;
 
@@ -282,7 +284,9 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
 		mopt->m_len = sizeof(int);
 		ip = mtod(mopt, int *);
 		*ip = IP_PORTRANGE_DEFAULT;
+		s = solock(so);
 		error = sosetopt(so, IPPROTO_IP, IP_PORTRANGE, mopt);
+		sounlock(s);
 		if (error)
 			goto bad;
 	}
@@ -331,6 +335,7 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
 	 * Always set receive timeout to detect server crash and reconnect.
 	 * Otherwise, we can get stuck in soreceive forever.
 	 */
+	s = solock(so);
 	so->so_rcv.sb_timeo = (5 * hz);
 	if (nmp->nm_flag & (NFSMNT_SOFT | NFSMNT_INT))
 		so->so_snd.sb_timeo = (5 * hz);
@@ -364,7 +369,6 @@ nfs_connect(struct nfsmount *nmp, struct nfsreq *rep)
 		rcvreserve = (nmp->nm_rsize + NFS_MAXPKTHDR +
 		    sizeof (u_int32_t)) * 2;
 	}
-	s = solock(so);
 	error = soreserve(so, sndreserve, rcvreserve);
 	sounlock(s);
 	if (error)
diff --git a/sys/nfs/nfs_syscalls.c b/sys/nfs/nfs_syscalls.c
index 036d81ab761..2e92ecad6c6 100644
--- a/sys/nfs/nfs_syscalls.c
+++ b/sys/nfs/nfs_syscalls.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: nfs_syscalls.c,v 1.109 2017/06/27 12:02:43 mpi Exp $	*/
+/*	$OpenBSD: nfs_syscalls.c,v 1.110 2017/08/09 14:22:58 mpi 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;
 	s = solock(so);
 	error = soreserve(so, siz, siz); 
-	sounlock(s);
 	if (error) {
+		sounlock(s);
 		m_freem(mynam);
 		return (error);
 	}
@@ -277,11 +277,11 @@ nfssvc_addsock(struct file *fp, struct mbuf *mynam)
 	so->so_rcv.sb_timeo = 0;
 	so->so_snd.sb_flags &= ~SB_NOINTR;
 	so->so_snd.sb_timeo = 0;
+	sounlock(s);
 	if (tslp)
 		slp = tslp;
 	else {
-		slp = malloc(sizeof(*slp), M_NFSSVC,
-		    M_WAITOK|M_ZERO);
+		slp = malloc(sizeof(*slp), M_NFSSVC, M_WAITOK|M_ZERO);
 		TAILQ_INSERT_TAIL(&nfssvc_sockhead, slp, ns_chain);
 	}
 	slp->ns_so = so;