From: Alexander Bluhm <bluhm@openbsd.org>
Subject: socket splicing oob inline race
To: tech@openbsd.org
Date: Wed, 22 Jan 2025 14:59:11 +0100

Hi,

Socket slicing test regress/sys/kern/sosplice/tcp run-args-oobinline.pl
failed from time to time.  It is splicing out-of-bound data that
is inline within the TCP stream.  The test reported wrong offsets
in the TCP urgent pointer.

Problem is that length of receive buffer and so_oobmark are not
modified in the same critical section.  To call pru_rcvd() the
receive socket buffer is released.  Moving mutex leave and calling
pru_rcvd() after updating so_oobmark fixes the test.

When somove() was holding exclusive net lock, order did not matter.

ok?

bluhm

Index: kern/uipc_socket.c
===================================================================
RCS file: /data/mirror/openbsd/cvs/src/sys/kern/uipc_socket.c,v
diff -u -p -U5 -r1.362 uipc_socket.c
--- kern/uipc_socket.c	21 Jan 2025 17:41:39 -0000	1.362
+++ kern/uipc_socket.c	22 Jan 2025 13:35:20 -0000
@@ -1755,31 +1755,31 @@ somove(struct socket *so, int wait)
 	if (m->m_flags & M_PKTHDR) {
 		m_resethdr(m);
 		m->m_pkthdr.len = len;
 	}
 
-	/* Send window update to source peer as receive buffer has changed. */
-	if (so->so_proto->pr_flags & PR_WANTRCVD) {
-		mtx_leave(&sosp->so_snd.sb_mtx);
-		mtx_leave(&so->so_rcv.sb_mtx);
-		solock_shared(so);
-		pru_rcvd(so);
-		sounlock_shared(so);
-		mtx_enter(&so->so_rcv.sb_mtx);
-		mtx_enter(&sosp->so_snd.sb_mtx);
-	}
-
 	/* Receive buffer did shrink by len bytes, adjust oob. */
 	rcvstate = so->so_rcv.sb_state;
 	so->so_rcv.sb_state &= ~SS_RCVATMARK;
 	oobmark = so->so_oobmark;
 	so->so_oobmark = oobmark > len ? oobmark - len : 0;
 	if (oobmark) {
 		if (oobmark == len)
 			so->so_rcv.sb_state |= SS_RCVATMARK;
 		if (oobmark >= len)
 			oobmark = 0;
+	}
+
+	/* Send window update to source peer as receive buffer has changed. */
+	if (so->so_proto->pr_flags & PR_WANTRCVD) {
+		mtx_leave(&sosp->so_snd.sb_mtx);
+		mtx_leave(&so->so_rcv.sb_mtx);
+		solock_shared(so);
+		pru_rcvd(so);
+		sounlock_shared(so);
+		mtx_enter(&so->so_rcv.sb_mtx);
+		mtx_enter(&sosp->so_snd.sb_mtx);
 	}
 
 	/*
 	 * Handle oob data.  If any malloc fails, ignore error.
 	 * TCP urgent data is not very reliable anyway.