From: Alexander Bluhm <bluhm@openbsd.org>
Subject: lock sonewconn() for internet sockets
To: tech@openbsd.org
Date: Thu, 16 Jan 2025 14:22:49 +0100

Hi,

I would like to lock the new socket in sonewconn() also for internet
family.  This is neeed for TCP input unlocking.  When we have to
lock two sockets, one needs shared netlock, but the other already
has it.  As this is the same idea as in doaccept() unify the lock
functions.

ok?

bluhm

Index: kern/uipc_socket2.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_socket2.c,v
diff -u -p -r1.164 uipc_socket2.c
--- kern/uipc_socket2.c	5 Jan 2025 12:36:48 -0000	1.164
+++ kern/uipc_socket2.c	16 Jan 2025 13:13:38 -0000
@@ -186,14 +186,8 @@ struct socket *
 sonewconn(struct socket *head, int connstatus, int wait)
 {
 	struct socket *so;
-	int persocket = solock_persocket(head);
 	int soqueue = connstatus ? 1 : 0;
 
-	/*
-	 * XXXSMP as long as `so' and `head' share the same lock, we
-	 * can call soreserve() and pr_attach() below w/o explicitly
-	 * locking `so'.
-	 */
 	soassertlocked(head);
 
 	if (m_pool_used() > 95)
@@ -218,8 +212,7 @@ sonewconn(struct socket *head, int conns
 	/*
 	 * Lock order will be `head' -> `so' while these sockets are linked.
 	 */
-	if (persocket)
-		solock(so);
+	solock_nonet(so);
 
 	/*
 	 * Inherit watermarks but those may get clamped in low mem situations.
@@ -252,14 +245,12 @@ sonewconn(struct socket *head, int conns
 		wakeup(&head->so_timeo);
 	}
 
-	if (persocket)
-		sounlock(so);
+	sounlock_nonet(so);
 
 	return (so);
 
 fail:
-	if (persocket)
-		sounlock(so);
+	sounlock_nonet(so);
 	sigio_free(&so->so_sigio);
 	klist_free(&so->so_rcv.sb_klist);
 	klist_free(&so->so_snd.sb_klist);
@@ -363,12 +354,21 @@ solock_shared(struct socket *so)
 	case PF_INET:
 	case PF_INET6:
 		NET_LOCK_SHARED();
-		rw_enter_write(&so->so_lock);
 		break;
-	default:
-		rw_enter_write(&so->so_lock);
+	}
+	rw_enter_write(&so->so_lock);
+}
+
+void
+solock_nonet(struct socket *so)
+{
+	switch (so->so_proto->pr_domain->dom_family) {
+	case PF_INET:
+	case PF_INET6:
+		NET_ASSERT_LOCKED();
 		break;
 	}
+	rw_enter_write(&so->so_lock);
 }
 
 int
@@ -416,16 +416,19 @@ sounlock(struct socket *so)
 void
 sounlock_shared(struct socket *so)
 {
+	rw_exit_write(&so->so_lock);
 	switch (so->so_proto->pr_domain->dom_family) {
 	case PF_INET:
 	case PF_INET6:
-		rw_exit_write(&so->so_lock);
 		NET_UNLOCK_SHARED();
 		break;
-	default:
-		rw_exit_write(&so->so_lock);
-		break;
 	}
+}
+
+void
+sounlock_nonet(struct socket *so)
+{
+	rw_exit_write(&so->so_lock);
 }
 
 void
Index: kern/uipc_syscalls.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_syscalls.c,v
diff -u -p -r1.221 uipc_syscalls.c
--- kern/uipc_syscalls.c	5 Jan 2025 11:33:45 -0000	1.221
+++ kern/uipc_syscalls.c	16 Jan 2025 13:01:34 -0000
@@ -247,34 +247,6 @@ sys_accept4(struct proc *p, void *v, reg
 	    SCARG(uap, anamelen), SCARG(uap, flags), retval));
 }
 
-void
-doaccept_solock(struct socket *so, int take_netlock)
-{
-	if (take_netlock) {
-		switch (so->so_proto->pr_domain->dom_family) {
-		case PF_INET:
-		case PF_INET6:
-			NET_LOCK_SHARED();
-		}
-	}
-
-	rw_enter_write(&so->so_lock);
-}
-
-void
-doaccept_sounlock(struct socket *so, int release_netlock)
-{
-	rw_exit_write(&so->so_lock);
-
-	if (release_netlock) {
-		switch (so->so_proto->pr_domain->dom_family) {
-		case PF_INET:
-		case PF_INET6:
-			NET_UNLOCK_SHARED();
-		}
-	}
-}
-
 int
 doaccept(struct proc *p, int sock, struct sockaddr *name, socklen_t *anamelen,
     int flags, register_t *retval)
@@ -307,7 +279,7 @@ doaccept(struct proc *p, int sock, struc
 	nam = m_get(M_WAIT, MT_SONAME);
 
 	head = headfp->f_data;
-	doaccept_solock(head, 1);
+	solock_shared(head);
 
 	if (isdnssocket(head) || (head->so_options & SO_ACCEPTCONN) == 0) {
 		error = EINVAL;
@@ -341,7 +313,7 @@ doaccept(struct proc *p, int sock, struc
 	 */
 	so = TAILQ_FIRST(&head->so_q);
 
-	doaccept_solock(so, 0);
+	solock_nonet(so);
 
 	if (soqremque(so, 1) == 0)
 		panic("accept");
@@ -353,7 +325,7 @@ doaccept(struct proc *p, int sock, struc
 	/* connection has been removed from the listen queue */
 	knote(&head->so_rcv.sb_klist, 0);
 
-	doaccept_sounlock(head, 0);
+	sounlock_nonet(head);
 
 	fp->f_type = DTYPE_SOCKET;
 	fp->f_flag = FREAD | FWRITE | nflag;
@@ -362,7 +334,7 @@ doaccept(struct proc *p, int sock, struc
 
 	error = soaccept(so, nam);
 
-	doaccept_sounlock(so, 1);
+	sounlock_shared(so);
 
 	if (error)
 		goto out;
@@ -385,7 +357,7 @@ doaccept(struct proc *p, int sock, struc
 	return 0;
 
 out_unlock:
-	doaccept_sounlock(head, 1);
+	sounlock_shared(head);
 out:
 	fdplock(fdp);
 	fdremove(fdp, tmpfd);
Index: sys/socketvar.h
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/sys/socketvar.h,v
diff -u -p -r1.138 socketvar.h
--- sys/socketvar.h	3 Jan 2025 12:56:15 -0000	1.138
+++ sys/socketvar.h	16 Jan 2025 13:01:34 -0000
@@ -451,10 +451,12 @@ int	sockargs(struct mbuf **, const void 
 int	sosleep_nsec(struct socket *, void *, int, const char *, uint64_t);
 void	solock(struct socket *);
 void	solock_shared(struct socket *);
+void	solock_nonet(struct socket *);
 int	solock_persocket(struct socket *);
 void	solock_pair(struct socket *, struct socket *);
 void	sounlock(struct socket *);
 void	sounlock_shared(struct socket *);
+void	sounlock_nonet(struct socket *);
 
 int	sendit(struct proc *, int, struct msghdr *, int, register_t *);
 int	recvit(struct proc *, int, struct msghdr *, caddr_t, register_t *);