Source-Changes-HG archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

[src/trunk]: src/sys/dev sequencer(4): Fix lock leak in ioctl(FIOASYNC).



details:   https://anonhg.NetBSD.org/src/rev/72d6bd8f453b
branches:  trunk
changeset: 368236:72d6bd8f453b
user:      riastradh <riastradh%NetBSD.org@localhost>
date:      Fri Jul 01 01:04:59 2022 +0000

description:
sequencer(4): Fix lock leak in ioctl(FIOASYNC).

Reported-by: syzbot+0bfe9054ab0e70ab02e6%syzkaller.appspotmail.com@localhost
Reported-by: syzbot+f2c5459483e60dd2c877%syzkaller.appspotmail.com@localhost
Reported-by: syzbot+d932ddf0ec7e30784ca8%syzkaller.appspotmail.com@localhost

Note: The evidence from
https://syzkaller.appspot.com/bug?id=19e1c8779bb0e020188ca80a586e470b59d7525d
suggests in the stack trace that the syscall was 16, which is chown.
This is not accurate; it happens because the x86 syscall logic sets
rax in the trapframe to the return value (in this case, 16 is EBUSY)
_before_ calling userret which does the lockdebug checks, and ddb
uses the trapframe's rax to show what the syscall number was when
printing its stack trace:

[ 104.0605877] Mutex error: lwp_exit,1084: sleep lock held
[ 104.0605877] lock address : 0xffffd9801269e050 type     :     sleep/adaptive
...
[ 104.5005727] syscall() at netbsd:syscall+0x323 sys/arch/x86/x86/syscall.c:92
[ 104.5115909] --- syscall (number 16) ---

XXX Maybe we should record the syscall number elsewhere so this is
not so confusing -- second time in 24h I've been bitten by this, and
in a different way.

diffstat:

 sys/dev/sequencer.c |  10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diffs (31 lines):

diff -r 1edc5c7e7d4f -r 72d6bd8f453b sys/dev/sequencer.c
--- a/sys/dev/sequencer.c       Fri Jul 01 01:04:01 2022 +0000
+++ b/sys/dev/sequencer.c       Fri Jul 01 01:04:59 2022 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: sequencer.c,v 1.80 2022/06/04 03:31:10 pgoyette Exp $  */
+/*     $NetBSD: sequencer.c,v 1.81 2022/07/01 01:04:59 riastradh Exp $ */
 
 /*
  * Copyright (c) 1998, 2008 The NetBSD Foundation, Inc.
@@ -55,7 +55,7 @@
  */
 
 #include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: sequencer.c,v 1.80 2022/06/04 03:31:10 pgoyette Exp $");
+__KERNEL_RCSID(0, "$NetBSD: sequencer.c,v 1.81 2022/07/01 01:04:59 riastradh Exp $");
 
 #ifdef _KERNEL_OPT
 #include "midi.h"
@@ -744,8 +744,10 @@
 
        case FIOASYNC:
                if (*(int *)addr) {
-                       if (sc->async != 0)
-                               return EBUSY;
+                       if (sc->async != 0) {
+                               error = EBUSY;
+                               break;
+                       }
                        sc->async = curproc->p_pid;
                        DPRINTF(("%s: FIOASYNC %d\n", __func__,
                            sc->async));



Home | Main Index | Thread Index | Old Index