From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756573AbcH2HHR (ORCPT ); Mon, 29 Aug 2016 03:07:17 -0400 Received: from mx2.suse.de ([195.135.220.15]:51725 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756489AbcH2HHP (ORCPT ); Mon, 29 Aug 2016 03:07:15 -0400 Date: Mon, 29 Aug 2016 09:07:14 +0200 Message-ID: From: Takashi Iwai To: "Vegard Nossum" Cc: "Jaroslav Kysela" , , "syzkaller" , Subject: Re: [PATCH 3/3] ALSA: timer: fix NULL pointer dereference on memory allocation failure In-Reply-To: <20160828223351.32489-3-vegard.nossum@oracle.com> References: <20160828223351.32489-1-vegard.nossum@oracle.com> <20160828223351.32489-3-vegard.nossum@oracle.com> User-Agent: Wanderlust/2.15.9 (Almost Unreal) SEMI/1.14.6 (Maruoka) FLIM/1.14.9 (=?UTF-8?B?R29qxY0=?=) APEL/10.8 Emacs/24.5 (x86_64-suse-linux-gnu) MULE/6.0 (HANACHIRUSATO) MIME-Version: 1.0 (generated by SEMI 1.14.6 - "Maruoka") Content-Type: text/plain; charset=US-ASCII Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 29 Aug 2016 00:33:51 +0200, Vegard Nossum wrote: > > I hit this with syzkaller: > > kasan: CONFIG_KASAN_INLINE enabled > kasan: GPF could be caused by NULL-ptr deref or user memory access > general protection fault: 0000 [#1] PREEMPT SMP KASAN > CPU: 0 PID: 1327 Comm: a.out Not tainted 4.8.0-rc2+ #190 > Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.9.3-0-ge2fc41e-prebuilt.qemu-project.org 04/01/2014 > task: ffff88011278d600 task.stack: ffff8801120c0000 > RIP: 0010:[] [] snd_hrtimer_start+0x77/0x100 > RSP: 0018:ffff8801120c7a60 EFLAGS: 00010006 > RAX: dffffc0000000000 RBX: 0000000000000000 RCX: 0000000000000007 > RDX: 0000000000000009 RSI: 1ffff10023483091 RDI: 0000000000000048 > RBP: ffff8801120c7a78 R08: ffff88011a5cf768 R09: ffff88011a5ba790 > R10: 0000000000000002 R11: ffffed00234b9ef1 R12: ffff880114843980 > R13: ffffffff84213c00 R14: ffff880114843ab0 R15: 0000000000000286 > FS: 00007f72958f3700(0000) GS:ffff88011aa00000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000603001 CR3: 00000001126ab000 CR4: 00000000000006f0 > Stack: > ffff880114843980 ffff880111eb2dc0 ffff880114843a34 ffff8801120c7ad0 > ffffffff82c81ab1 0000000000000000 ffffffff842138e0 0000000100000000 > ffff880111eb2dd0 ffff880111eb2dc0 0000000000000001 ffff880111eb2dc0 > Call Trace: > [] snd_timer_start1+0x331/0x670 > [] snd_timer_start+0x5d/0xa0 > [] snd_timer_user_ioctl+0x88e/0x2830 > [] ? __follow_pte.isra.49+0x430/0x430 > [] ? snd_timer_pause+0x80/0x80 > [] ? do_wp_page+0x3aa/0x1c90 > [] ? put_prev_entity+0x108f/0x21a0 > [] ? snd_timer_pause+0x80/0x80 > [] do_vfs_ioctl+0x193/0x1050 > [] ? cpuacct_account_field+0x12f/0x1a0 > [] ? ioctl_preallocate+0x200/0x200 > [] ? syscall_trace_enter+0x3cf/0xdb0 > [] ? __context_tracking_exit.part.4+0x9a/0x1e0 > [] ? exit_to_usermode_loop+0x190/0x190 > [] ? check_preemption_disabled+0x37/0x1e0 > [] ? security_file_ioctl+0x89/0xb0 > [] SyS_ioctl+0x8f/0xc0 > [] ? do_vfs_ioctl+0x1050/0x1050 > [] do_syscall_64+0x1c4/0x4e0 > [] entry_SYSCALL64_slow_path+0x25/0x25 > Code: c7 c7 c4 b9 c8 82 48 89 d9 4c 89 ee e8 63 88 7f fe e8 7e 46 7b fe 48 8d 7b 48 48 b8 00 00 00 00 00 fc ff df 48 89 fa 48 c1 ea 03 <0f> b6 04 02 84 c0 74 04 84 c0 7e 65 80 7b 48 00 74 0e e8 52 46 > RIP [] snd_hrtimer_start+0x77/0x100 > RSP > ---[ end trace 5955b08db7f2b029 ]--- > > This can happen if snd_hrtimer_open() fails to allocate memory and > returns an error, which is currently not checked by snd_timer_open(): > > ioctl(SNDRV_TIMER_IOCTL_SELECT) > - snd_timer_user_tselect() > - snd_timer_close() > - snd_hrtimer_close() > - (struct snd_timer *) t->private_data = NULL > - snd_timer_open() > - snd_hrtimer_open() > - kzalloc() fails; t->private_data is still NULL > > ioctl(SNDRV_TIMER_IOCTL_START) > - snd_timer_user_start() > - snd_timer_start() > - snd_timer_start1() > - snd_hrtimer_start() > - t->private_data == NULL // boom > > Signed-off-by: Vegard Nossum Applied this one with Cc to stable, too. Thanks. Takashi > --- > sound/core/timer.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/sound/core/timer.c b/sound/core/timer.c > index faa18f4..056d757 100644 > --- a/sound/core/timer.c > +++ b/sound/core/timer.c > @@ -294,8 +294,21 @@ int snd_timer_open(struct snd_timer_instance **ti, > get_device(&timer->card->card_dev); > timeri->slave_class = tid->dev_sclass; > timeri->slave_id = slave_id; > - if (list_empty(&timer->open_list_head) && timer->hw.open) > - timer->hw.open(timer); > + > + if (list_empty(&timer->open_list_head) && timer->hw.open) { > + int err = timer->hw.open(timer); > + if (err) { > + kfree(timeri->owner); > + kfree(timeri); > + > + if (timer->card) > + put_device(&timer->card->card_dev); > + module_put(timer->module); > + mutex_unlock(®ister_mutex); > + return err; > + } > + } > + > list_add_tail(&timeri->open_list, &timer->open_list_head); > snd_timer_check_master(timeri); > mutex_unlock(®ister_mutex); > -- > 2.10.0.rc0.1.g07c9292 > >