From: mvs <mvs@openbsd.org>
Date: Thu, 13 May 2021 19:43:11 +0000 (+0000)
Subject: Do `so_rcv' cleanup with sblock() held.
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=c3ed56ccf745fb861274d4b4ccdeca2d0a369291;p=openbsd

Do `so_rcv' cleanup with sblock() held.

solock() should be taken before sblock(). soreceive() grabs solock() and
then locks `so_rcv'. But later it releases solock() before call uimove(9).
So concurrent thread which performs soshutdown() could break sorecive()
loop. But `so_rcv' is still locked by sblock() so this soshutdown()
thread will sleep in sorflush() at sblock() call. soshutdown() thread
doesn't release solock() after sblock() call so it has no matter where to
release `so_rcv' - is will be locked until the solock() release.

That's why this strange looking code works fine. This sbunlock() movement
just after `so_rcv' cleanup  affects nothing but makes the code
consistent and clean to understand.

ok mpi@
---

diff --git a/sys/kern/uipc_socket.c b/sys/kern/uipc_socket.c
index 1dd03615ce0..341b9ae053a 100644
--- a/sys/kern/uipc_socket.c
+++ b/sys/kern/uipc_socket.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: uipc_socket.c,v 1.260 2021/05/13 18:06:54 mvs Exp $	*/
+/*	$OpenBSD: uipc_socket.c,v 1.261 2021/05/13 19:43:11 mvs Exp $	*/
 /*	$NetBSD: uipc_socket.c,v 1.21 1996/02/04 02:17:52 christos Exp $	*/
 
 /*
@@ -1127,11 +1127,11 @@ sorflush(struct socket *so)
 	/* with SB_NOINTR and M_WAITOK sblock() must not fail */
 	KASSERT(error == 0);
 	socantrcvmore(so);
-	sbunlock(so, sb);
 	m = sb->sb_mb;
 	memset(&sb->sb_startzero, 0,
 	     (caddr_t)&sb->sb_endzero - (caddr_t)&sb->sb_startzero);
 	sb->sb_timeo_nsecs = INFSLP;
+	sbunlock(so, sb);
 	if (pr->pr_flags & PR_RIGHTS && pr->pr_domain->dom_dispose)
 		(*pr->pr_domain->dom_dispose)(m);
 	m_purge(m);