Committed, thanks!
On Sat, Jul 02, 2022 at 08:50:33AM +0200, Anton Lindqvist wrote:
> On Fri, Jul 01, 2022 at 03:56:18AM -0600, Vitaliy Makkoveev wrote:
> > CVSROOT: /cvs
> > Module name: src
> > Changes by: m...@cvs.openbsd.org 2022/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=10cffe24080000
> 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=15cc61f4080000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=178e44ec080000
>
> 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 (0xfffffd8073dde420)
> #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
