linux-kernel - Re: WARNING in snd_timer_user_info

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <s5hmv3fvdt3.wl-tiwai@suse.de>
Date:   Tue, 21 Nov 2017 16:42:16 +0100
From:   Takashi Iwai <tiwai@...e.de>
To:     syzbot 
        <bot+fa48f938d494498bd7c8fb24bc99a3a30a1d0677@...kaller.appspotmail.com>
Cc:     <alsa-devel@...a-project.org>, <syzkaller-bugs@...glegroups.com>,
        <perex@...ex.cz>, <linux-kernel@...r.kernel.org>
Subject: Re: WARNING in snd_timer_user_info_compat

On Tue, 21 Nov 2017 16:27:58 +0100,
syzbot wrote:
> 
> Hello,
> 
> syzkaller hit the following crash on
> c8a0739b185d11d6e2ca7ad9f5835841d1cfc765
> git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/master
> compiler: gcc (GCC) 7.1.1 20170620
> .config is attached
> Raw console output is attached.
> C reproducer is attached
> syzkaller reproducer is attached. See https://goo.gl/kgGztJ
> for information about syzkaller reproducers
> 
> 
> Kernel panic - not syncing: panic_on_warn set ...
> 
> CPU: 1 PID: 3039 Comm: syzkaller881964 Not tainted 4.14.0+ #98
> Hardware name: Google Google Compute Engine/Google Compute Engine,
> BIOS  Google 01/01/2011
> Call Trace:
>  __dump_stack lib/dump_stack.c:17 [inline]
>  dump_stack+0x194/0x257 lib/dump_stack.c:53
>  panic+0x1e4/0x41c kernel/panic.c:183
>  __warn+0x1dc/0x200 kernel/panic.c:547
>  report_bug+0x211/0x2d0 lib/bug.c:184
>  fixup_bug.part.11+0x37/0x80 arch/x86/kernel/traps.c:177
>  fixup_bug arch/x86/kernel/traps.c:246 [inline]
>  do_error_trap+0x2d7/0x3e0 arch/x86/kernel/traps.c:295
>  do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:314
>  invalid_op+0x18/0x20 arch/x86/entry/entry_64.S:926
> RIP: 0010:snd_timer_user_info_compat.isra.6+0x3be/0x490
> sound/core/timer_compat.c:69

Aha, it's a superfluous WARN_ON() usage from snd_BUG_ON().
Now syzkaller turned on CONFIG_SND_DEBUG=y, so this surfaced.
The fix patch is below.  I'm going to queue this.

#syz fix: ALSA: timer: Remove kernel warning at compat ioctl error paths


thanks,

Takashi

---
From: Takashi Iwai <tiwai@...e.de>
Subject: [PATCH] ALSA: timer: Remove kernel warning at compat ioctl error
 paths

Some timer compat ioctls have NULL checks of timer instance with
snd_BUG_ON() that bring up WARN_ON() when the debug option is set.
Actually the condition can be met in the normal situation and it's
confusing and bad to spew kernel warnings with stack trace there.
Let's remove snd_BUG_ON() invocation and replace with the simple
checks.  Also, correct the error code to EBADFD to follow the native
ioctl error handling.

Reported-by: syzbot <syzkaller@...glegroups.com>
Cc: <stable@...r.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@...e.de>
---
 sound/core/timer_compat.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/core/timer_compat.c b/sound/core/timer_compat.c
index 59127b6ef39e..e00f7e399e46 100644
--- a/sound/core/timer_compat.c
+++ b/sound/core/timer_compat.c
@@ -66,11 +66,11 @@ static int snd_timer_user_info_compat(struct file *file,
 	struct snd_timer *t;
 
 	tu = file->private_data;
-	if (snd_BUG_ON(!tu->timeri))
-		return -ENXIO;
+	if (!tu->timeri)
+		return -EBADFD;
 	t = tu->timeri->timer;
-	if (snd_BUG_ON(!t))
-		return -ENXIO;
+	if (!t)
+		return -EBADFD;
 	memset(&info, 0, sizeof(info));
 	info.card = t->card ? t->card->number : -1;
 	if (t->hw.flags & SNDRV_TIMER_HW_SLAVE)
@@ -99,8 +99,8 @@ static int snd_timer_user_status_compat(struct file *file,
 	struct snd_timer_status32 status;
 	
 	tu = file->private_data;
-	if (snd_BUG_ON(!tu->timeri))
-		return -ENXIO;
+	if (!tu->timeri)
+		return -EBADFD;
 	memset(&status, 0, sizeof(status));
 	status.tstamp.tv_sec = tu->tstamp.tv_sec;
 	status.tstamp.tv_nsec = tu->tstamp.tv_nsec;
-- 
2.15.0