On Fri, Jul 01, 2022 at 03:56:18AM -0600, Vitaliy Makkoveev wrote:
> CVSROOT: /cvs
> Module name: src
> Changes by: m...@cvs.openbsd.org2022/07/01 03:56:18
>
> Modified files:
> sys/kern : uipc_socket.c uipc_socket2.c uipc_syscalls.c
>uipc_usrreq.c
> sys/miscfs/fifofs: fifo_vnops.c
> sys/sys: socketvar.h unpcb.h
>
> Log message:
> Make fine grained unix(4) domain sockets locking. Use the per-socket
> `so_lock' rwlock(9) instead of global `unp_lock' which locks the whole
> layer.
>
> The PCB of unix(4) sockets are linked to each other and we need to lock
> them both. This introduces the lock ordering problem, because when the
> thread (1) keeps lock on `so1' and trying to lock `so2', the thread (2)
> could hold lock on `so2' and trying to lock `so1'. To solve this we
> always lock sockets in the strict order.
>
> For the sockets which are already accessible from userland, we always
> lock socket with the smallest memory address first. Sometimes we need to
> unlock socket before lock it's peer and lock it again.
>
> We use reference counters for prevent the connected peer destruction
> during to relock. We also handle the case where the peer socket was
> replaced by another socket.
>
> For the newly connected sockets, which are not yet exported to the
> userland by accept(2), we always lock the listening socket `head' first.
> This allows us to avoid unwanted relock within accept(2) syscall.
>
> ok claudio@
syzkaller found a panic, I'm spotting one missed unlock. Feel free to
commit.
diff --git sys/kern/uipc_usrreq.c sys/kern/uipc_usrreq.c
index 0710393d376..7231b16b735 100644
--- sys/kern/uipc_usrreq.c
+++ sys/kern/uipc_usrreq.c
@@ -363,6 +363,7 @@ uipc_usrreq(struct socket *so, int req, struct mbuf *m,
struct mbuf *nam,
control = NULL;
} else {
error = ENOBUFS;
+ sounlock(so2);
break;
}
} else if (so->so_type == SOCK_SEQPACKET)
syzbot has found a reproducer for the following issue on:
HEAD commit:a4a82b864d54 Remove PIPEXCSESSION ioctl(2) call only from ..
git tree: openbsd
console output: https://syzkaller.appspot.com/x/log.txt?x=10cffe2408
kernel config: https://syzkaller.appspot.com/x/.config?x=7058272de1526588
dashboard link: https://syzkaller.appspot.com/bug?extid=a648408d6a58fd40b59a
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15cc61f408
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=178e44ec08
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+a648408d6a58fd40b...@syzkaller.appspotmail.com
witness: userret: returning with the following locks held:
exclusive rwlock solock r = 0 (0xfd8073dde420)
#0 witness_lock+0x44d
#1 unp_solock_peer+0x64 sys/kern/uipc_usrreq.c:168
#2 uipc_usrreq+0x7c6 sys/kern/uipc_usrreq.c:350
#3 sosend+0x61b sys/kern/uipc_socket.c:657
#4 sendit+0x65d sys/kern/uipc_syscalls.c:682
#5 sys_sendmsg+0x198 sys/kern/uipc_syscalls.c:589
#6 syscall+0x4c3 mi_syscall sys/sys/syscall_mi.h:101 [inline]
#6 syscall+0x4c3 sys/arch/amd64/amd64/trap.c:585
#7 Xsyscall+0x128
panic: witness_warn
Stopped at db_enter+0x18: addq$0x8,%rsp
TIDPIDUID PRFLAGS PFLAGS CPU COMMAND
* 35921 8907 0 0x2 00 syz-executor3905931047
442241 79361 73 0x1100010 01 syslogd
db_enter() at db_enter+0x18 sys/arch/amd64/amd64/db_interface.c:437
panic(82593e80) at panic+0x177 sys/kern/subr_prf.c:202
witness_warn(2,0,82620d13) at witness_warn+0x65e witness_debugger
sys/kern/subr_witness.c:2505 [inline]
witness_warn(2,0,82620d13) at witness_warn+0x65e
sys/kern/subr_witness.c:1473
userret(800021233268) at userret+0x265 sys/kern/kern_sig.c:2012
syscall(8000212825b0) at syscall+0x57e mi_syscall_return
sys/sys/syscall_mi.h:128 [inline]
syscall(8000212825b0) at syscall+0x57e sys/arch/amd64/amd64/trap.c:607
Xsyscall() at Xsyscall+0x128
end of kernel
end trace frame: 0x7f7f0b10, count: 9
https://www.openbsd.org/ddb.html describes the minimum info required in bug
reports. Insufficient info makes it difficult to find and fix bugs.
ddb{0}>
ddb{0}> set $lines = 0
ddb{0}> set $maxwidth = 0
ddb{0}> show panic
*cpu0: witness_warn
ddb{0}> trace
db_enter() at db_enter+0x18 sys/arch/amd64/amd64/db_interface.c:437
panic(82593e80) at panic+0x177 sys/kern/subr_prf.c:202
witness_warn(2,0,82620d13) at witness_warn+0x65e witness_debugger
sys/kern/subr_witness.c:2505 [inline]
witness_warn(2,0,82620d13) at witness_warn+0x65e
sys/kern/subr_witness.c:1473
userret(800021233268) at userret+0x265 sys/kern/kern_sig.c:2012
syscall(8000212825b0) at syscall+0x57e mi_syscal
