From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754474AbeDBVGR (ORCPT ); Mon, 2 Apr 2018 17:06:17 -0400 Received: from mx2.suse.de ([195.135.220.15]:51322 "EHLO mx2.suse.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754436AbeDBVGQ (ORCPT ); Mon, 2 Apr 2018 17:06:16 -0400 Date: Mon, 02 Apr 2018 23:06:14 +0200 Message-ID: From: Takashi Iwai To: Cc: , , , Subject: Re: KASAN: use-after-free Read in snd_pcm_timer_resolution In-Reply-To: <001a11427716f74ef40568951849@google.com> References: <001a11427716f74ef40568951849@google.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/25.3 (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 Fri, 30 Mar 2018 01:01:01 +0200, syzbot wrote: > > Hello, > > syzbot hit the following crash on upstream commit > 0b412605ef5f5c64b31f19e2910b1d5eba9929c3 (Thu Mar 29 01:07:23 2018 +0000) > Merge tag 'drm-fixes-for-v4.16-rc8' of > git://people.freedesktop.org/~airlied/linux > syzbot dashboard link: > https://syzkaller.appspot.com/bug?extid=8e62ff4e07aa2ce87826 > > So far this crash happened 3 times on upstream. > C reproducer: https://syzkaller.appspot.com/x/repro.c?id=6219387178582016 > syzkaller reproducer: > https://syzkaller.appspot.com/x/repro.syz?id=5176598244360192 > Raw console output: > https://syzkaller.appspot.com/x/log.txt?id=5815694612496384 > Kernel config: > https://syzkaller.appspot.com/x/.config?id=-8440362230543204781 > compiler: gcc (GCC) 7.1.1 20170620 > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+8e62ff4e07aa2ce87826@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. > > ================================================================== > BUG: KASAN: use-after-free in snd_pcm_timer_resolution+0xa3/0xc0 > sound/core/pcm_timer.c:72 > Read of size 4 at addr ffff8801bdd58490 by task syzkaller702213/13238 > > CPU: 0 PID: 13238 Comm: syzkaller702213 Not tainted 4.16.0-rc7+ #370 > 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/0x24d lib/dump_stack.c:53 > print_address_description+0x73/0x250 mm/kasan/report.c:256 > kasan_report_error mm/kasan/report.c:354 [inline] > kasan_report+0x23c/0x360 mm/kasan/report.c:412 > __asan_report_load4_noabort+0x14/0x20 mm/kasan/report.c:432 > snd_pcm_timer_resolution+0xa3/0xc0 sound/core/pcm_timer.c:72 > snd_timer_resolution sound/core/timer.c:439 [inline] > snd_timer_notify1+0x10f/0x400 sound/core/timer.c:462 > snd_timer_start1+0x586/0x8f0 sound/core/timer.c:522 > snd_timer_start+0x5d/0xa0 sound/core/timer.c:634 > snd_timer_user_start.isra.12+0xde/0x130 sound/core/timer.c:1878 > snd_timer_user_continue sound/core/timer.c:1902 [inline] > __snd_timer_user_ioctl+0x1771/0x2c10 sound/core/timer.c:1977 > snd_timer_user_ioctl+0x5f/0x7d sound/core/timer.c:1992 (snip) So this looks like a race between ALSA PCM and ALSA timer interfaces. When a PCM timer is accessed during closing the corresponding PCM substream, it may lead to such a UAF. The fix patch is below. I'll queue it after testing locally. thanks, Takashi -- 8< -- From: Takashi Iwai Subject: [PATCH] ALSA: pcm: Fix UAF at PCM release via PCM timer access The PCM runtime object is created and freed dynamically at PCM stream open / close time. This is tracked via substream->runtime, and it's cleared at snd_pcm_detach_substream(). The runtime object assignment is protected by PCM open_mutex, so for all PCM operations, it's safely handled. However, each PCM substream provides also an ALSA timer interface, and user-space can access to this while closing a PCM substream. This may eventually lead to a UAF, as snd_pcm_timer_resolution() tries to access the runtime while clearing it in other side. Fortunately, it's the only concurrent access from the PCM timer, and it merely reads runtime->timer_resolution field. So, we can avoid the race by reordering kfree() and wrapping the substream->runtime clearance with the corresponding timer lock. Reported-by: syzbot+8e62ff4e07aa2ce87826@syzkaller.appspotmail.com Cc: Signed-off-by: Takashi Iwai --- sound/core/pcm.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/sound/core/pcm.c b/sound/core/pcm.c index 09ee8c6b9f75..66ac89aad681 100644 --- a/sound/core/pcm.c +++ b/sound/core/pcm.c @@ -28,6 +28,7 @@ #include #include #include +#include #include #include @@ -1054,8 +1055,13 @@ void snd_pcm_detach_substream(struct snd_pcm_substream *substream) snd_free_pages((void*)runtime->control, PAGE_ALIGN(sizeof(struct snd_pcm_mmap_control))); kfree(runtime->hw_constraints.rules); - kfree(runtime); + /* Avoid concurrent access to runtime via PCM timer interface */ + if (substream->timer) + spin_lock_irq(&substream->timer->lock); substream->runtime = NULL; + if (substream->timer) + spin_unlock_irq(&substream->timer->lock); + kfree(runtime); put_pid(substream->pid); substream->pid = NULL; substream->pstr->substream_opened--; -- 2.16.2