Re: general protection fault in snd_rawmidi_ioctl_compat

[Takashi Iwai <tiwai@suse.de>]

On Thu, 19 Apr 2018 18:10:01 +0200,
syzbot wrote:
> 
> Hello,
> 
> syzbot hit the following crash on upstream commit
> b284d4d5a6785f8cd07eda2646a95782373cd01e (Tue Apr 10 19:25:30 2018 +0000)
> Merge tag 'ceph-for-4.17-rc1' of git://github.com/ceph/ceph-client
> syzbot dashboard link:
> https://syzkaller.appspot.com/bug?extid=f7a0348affc3b67bc617
> 
> Unfortunately, I don't have any reproducer for this crash yet.
> Raw console output:
> https://syzkaller.appspot.com/x/log.txt?id=5208906934517760
> Kernel config:
> https://syzkaller.appspot.com/x/.config?id=-1223000601505858474
> compiler: gcc (GCC) 8.0.1 20180301 (experimental)
> 
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+f7a0348affc3b67bc617@syzkaller.appspotmail.com
> It will help syzbot understand when the bug is fixed. See footer for
> details.
> If you forward the report, please keep this part and the footer.
> 
> kasan: CONFIG_KASAN_INLINE enabled
> kasan: GPF could be caused by NULL-ptr deref or user memory access
> general protection fault: 0000 [#1] SMP KASAN
> Dumping ftrace buffer:
>    (ftrace buffer empty)
> Modules linked in:
> CPU: 1 PID: 21671 Comm: syz-executor7 Not tainted 4.16.0+ #17
> Hardware name: Google Google Compute Engine/Google Compute Engine,
> BIOS  Google 01/01/2011
> RIP: 0010:snd_rawmidi_ioctl_status_x32 sound/core/rawmidi_compat.c:125
> [inline]
> RIP: 0010:snd_rawmidi_ioctl_compat+0x51d/0xba0
> sound/core/rawmidi_compat.c:168
> RSP: 0018:ffff88018ccdfc08 EFLAGS: 00010202
> RAX: dffffc0000000000 RBX: 0000000000000000 RCX: ffffc900054cb000
> RDX: 000000000000000b RSI: ffffffff85a61885 RDI: 0000000000000058
> RBP: ffff88018ccdfd80 R08: ffff8801af6f0740 R09: 0000000000000000
> R10: 0000000000000003 R11: 0000000000000000 R12: 1ffff1003199bf83
> R13: 0000000000930000 R14: ffff88018ccdfcf8 R15: 0000000000000001
> FS:  00007fa64d466700(0000) GS:ffff8801db100000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007fa30835b518 CR3: 00000001879ef000 CR4: 00000000001406e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
>  C_SYSC_ioctl fs/compat_ioctl.c:1461 [inline]
>  compat_SyS_ioctl+0xa37/0x33b0 fs/compat_ioctl.c:1407
>  do_syscall_64+0x29e/0x9d0 arch/x86/entry/common.c:287
>  entry_SYSCALL_64_after_hwframe+0x42/0xb7
> RIP: 0033:0x455259
> RSP: 002b:00007fa64d465c68 EFLAGS: 00000246 ORIG_RAX: 0000000000000202
> RAX: ffffffffffffffda RBX: 00007fa64d4666d4 RCX: 0000000000455259
> RDX: 0000000000930000 RSI: 00000000c0305720 RDI: 0000000000000013
> RBP: 000000000072bf58 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000020000180 R11: 0000000000000246 R12: 00000000ffffffff
> R13: 0000000000000641 R14: 00000000006fc6b8 R15: 0000000000000001
> Code: df 48 89 fa 48 c1 ea 03 80 3c 02 00 0f 85 1c 06 00 00 48 b8 00
> 00 00  00 00 fc ff df 48 8b 5b 08 48 8d 7b 58 48 89 fa 48 c1 ea 03
> <80> 3c 02 00  0f 85 e5 05 00 00 48 8b 7b 58 4c 89 f6 e8 5d cc ff
> RIP: snd_rawmidi_ioctl_status_x32 sound/core/rawmidi_compat.c:125
> [inline]  RSP: ffff88018ccdfc08
> RIP: snd_rawmidi_ioctl_compat+0x51d/0xba0
> sound/core/rawmidi_compat.c:168  RSP: ffff88018ccdfc08
> ---[ end trace 7dcd43c411f49842 ]---
> 
> 
> ---
> This bug is generated by a dumb bot. It may contain errors.
> See https://goo.gl/tpsmEJ for details.
> Direct all questions to syzkaller@googlegroups.com.
> 
> syzbot will keep track of this bug report.
> If you forgot to add the Reported-by tag, once the fix for this bug is
> merged
> into any tree, please reply to this email with:
> #syz fix: exact-commit-title
> To mark this as a duplicate of another syzbot report, please reply with:
> #syz dup: exact-subject-of-another-report
> If it's a one-off invalid bug report, please reply with:
> #syz invalid
> Note: if the crash happens again, it will cause creation of a new bug
> report.
> Note: all commands must start from beginning of the line in the email body.

Indeed we check only rfile->output but not rfile->input, so they may
pass NULL there.  The fix must be easy as below.


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: rawmidi: Fix missing input substream checks in compat
 ioctls

Some rawmidi compat ioctls lack of the input substream checks
(although they do check only for rfile->output).  This many eventually
lead to an Oops as NULL substream is passed to the rawmidi core
functions.

Fix it by adding the proper checks before each function call.

The bug was spotted by syzkaller.

Reported-by: syzbot+f7a0348affc3b67bc617@syzkaller.appspotmail.com
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/rawmidi_compat.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/sound/core/rawmidi_compat.c b/sound/core/rawmidi_compat.c
index f69764d7cdd7..e30e30ba6e39 100644
--- a/sound/core/rawmidi_compat.c
+++ b/sound/core/rawmidi_compat.c
@@ -36,8 +36,6 @@ static int snd_rawmidi_ioctl_params_compat(struct snd_rawmidi_file *rfile,
 	struct snd_rawmidi_params params;
 	unsigned int val;
 
-	if (rfile->output == NULL)
-		return -EINVAL;
 	if (get_user(params.stream, &src->stream) ||
 	    get_user(params.buffer_size, &src->buffer_size) ||
 	    get_user(params.avail_min, &src->avail_min) ||
@@ -46,8 +44,12 @@ static int snd_rawmidi_ioctl_params_compat(struct snd_rawmidi_file *rfile,
 	params.no_active_sensing = val;
 	switch (params.stream) {
 	case SNDRV_RAWMIDI_STREAM_OUTPUT:
+		if (!rfile->output)
+			return -EINVAL;
 		return snd_rawmidi_output_params(rfile->output, &params);
 	case SNDRV_RAWMIDI_STREAM_INPUT:
+		if (!rfile->input)
+			return -EINVAL;
 		return snd_rawmidi_input_params(rfile->input, &params);
 	}
 	return -EINVAL;
@@ -67,16 +69,18 @@ static int snd_rawmidi_ioctl_status_compat(struct snd_rawmidi_file *rfile,
 	int err;
 	struct snd_rawmidi_status status;
 
-	if (rfile->output == NULL)
-		return -EINVAL;
 	if (get_user(status.stream, &src->stream))
 		return -EFAULT;
 
 	switch (status.stream) {
 	case SNDRV_RAWMIDI_STREAM_OUTPUT:
+		if (!rfile->output)
+			return -EINVAL;
 		err = snd_rawmidi_output_status(rfile->output, &status);
 		break;
 	case SNDRV_RAWMIDI_STREAM_INPUT:
+		if (!rfile->input)
+			return -EINVAL;
 		err = snd_rawmidi_input_status(rfile->input, &status);
 		break;
 	default:
@@ -112,16 +116,18 @@ static int snd_rawmidi_ioctl_status_x32(struct snd_rawmidi_file *rfile,
 	int err;
 	struct snd_rawmidi_status status;
 
-	if (rfile->output == NULL)
-		return -EINVAL;
 	if (get_user(status.stream, &src->stream))
 		return -EFAULT;
 
 	switch (status.stream) {
 	case SNDRV_RAWMIDI_STREAM_OUTPUT:
+		if (!rfile->output)
+			return -EINVAL;
 		err = snd_rawmidi_output_status(rfile->output, &status);
 		break;
 	case SNDRV_RAWMIDI_STREAM_INPUT:
+		if (!rfile->input)
+			return -EINVAL;
 		err = snd_rawmidi_input_status(rfile->input, &status);
 		break;
 	default:
-- 
2.16.3