linux-kernel - Re: [PATCH 3/3] ALSA: timer: fix NULL pointer dereference on memory allocation failure

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <s5hoa4c834t.wl-tiwai@suse.de>
Date:   Mon, 29 Aug 2016 09:07:14 +0200
From:   Takashi Iwai <tiwai@...e.de>
To:     "Vegard Nossum" <vegard.nossum@...cle.com>
Cc:     "Jaroslav Kysela" <perex@...ex.cz>, <alsa-devel@...a-project.org>,
        "syzkaller" <syzkaller@...glegroups.com>,
        <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH 3/3] ALSA: timer: fix NULL pointer dereference on memory allocation failure

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:[<ffffffff82c8ba07>]  [<ffffffff82c8ba07>] 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:
>      [<ffffffff82c81ab1>] snd_timer_start1+0x331/0x670
>      [<ffffffff82c85bfd>] snd_timer_start+0x5d/0xa0
>      [<ffffffff82c8795e>] snd_timer_user_ioctl+0x88e/0x2830
>      [<ffffffff8159f3a0>] ? __follow_pte.isra.49+0x430/0x430
>      [<ffffffff82c870d0>] ? snd_timer_pause+0x80/0x80
>      [<ffffffff815a26fa>] ? do_wp_page+0x3aa/0x1c90
>      [<ffffffff8132762f>] ? put_prev_entity+0x108f/0x21a0
>      [<ffffffff82c870d0>] ? snd_timer_pause+0x80/0x80
>      [<ffffffff816b0733>] do_vfs_ioctl+0x193/0x1050
>      [<ffffffff813510af>] ? cpuacct_account_field+0x12f/0x1a0
>      [<ffffffff816b05a0>] ? ioctl_preallocate+0x200/0x200
>      [<ffffffff81002f2f>] ? syscall_trace_enter+0x3cf/0xdb0
>      [<ffffffff815045ba>] ? __context_tracking_exit.part.4+0x9a/0x1e0
>      [<ffffffff81002b60>] ? exit_to_usermode_loop+0x190/0x190
>      [<ffffffff82001a97>] ? check_preemption_disabled+0x37/0x1e0
>      [<ffffffff81d93889>] ? security_file_ioctl+0x89/0xb0
>      [<ffffffff816b167f>] SyS_ioctl+0x8f/0xc0
>      [<ffffffff816b15f0>] ? do_vfs_ioctl+0x1050/0x1050
>      [<ffffffff81005524>] do_syscall_64+0x1c4/0x4e0
>      [<ffffffff83c32b2a>] 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  [<ffffffff82c8ba07>] snd_hrtimer_start+0x77/0x100
>      RSP <ffff8801120c7a60>
>     ---[ 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 <vegard.nossum@...cle.com>

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(&register_mutex);
> +			return err;
> +		}
> +	}
> +
>  	list_add_tail(&timeri->open_list, &timer->open_list_head);
>  	snd_timer_check_master(timeri);
>  	mutex_unlock(&register_mutex);
> -- 
> 2.10.0.rc0.1.g07c9292
> 
>