Re: KASAN: use-after-free Read in snd_pcm_timer_resolution

[Takashi Iwai]

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@xxxxxxxxxxxxxxxxxxxxxxxxx
> 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 <tiwai@xxxxxxx>
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@xxxxxxxxxxxxxxxxxxxxxxxxx
Cc: <stable@xxxxxxxxxxxxxxx>
Signed-off-by: Takashi Iwai <tiwai@xxxxxxx>
---
 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 <sound/core.h>
 #include <sound/minors.h>
 #include <sound/pcm.h>
+#include <sound/timer.h>
 #include <sound/control.h>
 #include <sound/info.h>
 
@@ -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