From: Vitaliy Makkoveev <mvs@openbsd.org>
Subject: Don't unlock vnode(9) in fifo_read() and fifo_write()
To: tech@openbsd.org
Date: Tue, 16 Apr 2024 16:15:05 +0300

I suspect syzkaller reported panics [1] and [2] happened because sockets
were closed due vnode(9) lock release.

The solock() designed to be take after vnode(9) lock, and unix(4)
sockets perform solock() release for that purpose. No reason to release
vnode(9) lock in fifo_read() and fifo_write() around soreceive() and
sosend() respectively.

I can't reproduce syzkaller panics, so I can't say the issues were
fixed, but the unnecessary re-locking is the bad thing in any case.

I performed regress/sys/fifofs/ and regress/sys/kern/unixsockets/ test
without witness(4) warnings.

1. https://syzkaller.appspot.com/bug?extid=5779bc64fc4fdd0a5140
2. https://syzkaller.appspot.com/bug?extid=b7ef44521c90d0740ba3

Index: sys/miscfs/fifofs/fifo_vnops.c
===================================================================
RCS file: /cvs/src/sys/miscfs/fifofs/fifo_vnops.c,v
retrieving revision 1.104
diff -u -p -r1.104 fifo_vnops.c
--- sys/miscfs/fifofs/fifo_vnops.c	26 Mar 2024 09:46:47 -0000	1.104
+++ sys/miscfs/fifofs/fifo_vnops.c	16 Apr 2024 13:04:07 -0000
@@ -252,9 +252,7 @@ fifo_read(void *v)
 		return (0);
 	if (ap->a_ioflag & IO_NDELAY)
 		flags |= MSG_DONTWAIT;
-	VOP_UNLOCK(ap->a_vp);
 	error = soreceive(rso, NULL, uio, NULL, NULL, &flags, 0);
-	vn_lock(ap->a_vp, LK_EXCLUSIVE | LK_RETRY);
 	if (ap->a_ioflag & IO_NDELAY) {
 		if (error == EWOULDBLOCK &&
 		    ap->a_vp->v_fifoinfo->fi_writers == 0)
@@ -271,7 +269,7 @@ fifo_write(void *v)
 {
 	struct vop_write_args *ap = v;
 	struct socket *wso = ap->a_vp->v_fifoinfo->fi_writesock;
-	int error, flags = 0;
+	int flags = 0;
 
 #ifdef DIAGNOSTIC
 	if (ap->a_uio->uio_rw != UIO_WRITE)
@@ -279,10 +277,7 @@ fifo_write(void *v)
 #endif
 	if (ap->a_ioflag & IO_NDELAY)
 		flags |= MSG_DONTWAIT;
-	VOP_UNLOCK(ap->a_vp);
-	error = sosend(wso, NULL, ap->a_uio, NULL, NULL, flags);
-	vn_lock(ap->a_vp, LK_EXCLUSIVE | LK_RETRY);
-	return (error);
+	return sosend(wso, NULL, ap->a_uio, NULL, NULL, flags);
 }
 
 /*