All of lore.kernel.org
 help / color / mirror / Atom feed
* sound: use-after-free in hrtimer_cancel
@ 2016-06-04 18:00 Dmitry Vyukov
  2016-06-04 18:27 ` Dmitry Vyukov
  0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Vyukov @ 2016-06-04 18:00 UTC (permalink / raw)
  To: Takashi Iwai, Jaroslav Kysela, alsa-devel, LKML
  Cc: Sasha Levin, Alexander Potapenko, Kostya Serebryany, syzkaller

Hello,

The following program triggers use-after-free:

// autogenerated by syzkaller (http://github.com/google/syzkaller)
#include <pthread.h>
#include <stdint.h>
#include <string.h>
#include <sys/syscall.h>
#include <unistd.h>

long r[20];

void* thr(void* arg)
{
  switch ((long)arg) {
  case 0:
    break;
  case 1:
    r[2] = syscall(SYS_open, "/dev/audio", 0xa40ul, 0, 0, 0);
    break;
  case 2:
    *(uint32_t*)0x2001dde8 = r[2];
    *(uint16_t*)0x2001ddec = (uint16_t)0x0;
    *(uint16_t*)0x2001ddee = (uint16_t)0x2;
    *(uint32_t*)0x2001ddf0 = r[2];
    *(uint16_t*)0x2001ddf4 = (uint16_t)0xfffffffff3a15aea;
    *(uint16_t*)0x2001ddf6 = (uint16_t)0x1;
    *(uint32_t*)0x2001ddf8 = r[2];
    *(uint16_t*)0x2001ddfc = (uint16_t)0x836a;
    *(uint16_t*)0x2001ddfe = (uint16_t)0x1ff;
    *(uint32_t*)0x2001de00 = r[2];
    *(uint16_t*)0x2001de04 = (uint16_t)0x0;
    *(uint16_t*)0x2001de06 = (uint16_t)0x20;
    r[15] = syscall(SYS_poll, 0x2001dde8ul, 0x4ul, 0x8ul, 0, 0, 0);
    break;
  case 3:
    *(uint64_t*)0x20dc13c0 = (uint64_t)0x20dc1f27;
    *(uint64_t*)0x20dc13c8 = (uint64_t)0xd9;
    r[18] = syscall(SYS_readv, r[2], 0x20dc13c0ul, 0x1ul, 0, 0, 0);
    break;
  case 4:
    r[19] = syscall(SYS_read, r[2], 0x20dbefe0ul, 0x20ul, 0, 0, 0);
    break;
  }
  return 0;
}

int main()
{
  long i;
  pthread_t th[10];

  syscall(SYS_mmap, 0x20000000ul, 0xde0000ul, 0x3ul, 0x32ul,
                   0xfffffffffffffffful, 0x0ul);
  srand(getpid());
  memset(r, -1, sizeof(r));
  for (i = 0; i < 5; i++) {
    pthread_create(&th[i], 0, thr, (void*)i);
    usleep(rand()%5000);
  }
  for (i = 0; i < 5; i++) {
    pthread_create(&th[5+i], 0, thr, (void*)i);
    if (rand()%2)
      usleep(rand()%5000);
  }
  for (i = 0; i < 10; i++) {
    pthread_join(th[i], 0);
  }
  return 0;
}


Here are 2 reports (note they are slightly different):

==================================================================
BUG: KASAN: use-after-free in rb_erase+0x1b17/0x2010 at addr ffff88005e5b6f68
 Read of size 8 by task syz-executor/8984
=============================================================================
BUG kmalloc-192 (Not tainted): kasan: bad access detected
-----------------------------------------------------------------------------

Disabling lock debugging due to kernel taint
INFO: Allocated in 0xbbbbbbbbbbbbbbbb age=18446705582212484632
cpu=2172593693 pid=-1
[<     inline     >] kmalloc include/linux/slab.h:478
[<     inline     >] kzalloc include/linux/slab.h:622
[<      none      >] dummy_hrtimer_create+0x49/0x1a0 sound/drivers/dummy.c:464
[<      none      >] ___slab_alloc+0x55d/0x5a0 mm/slub.c:2476
[<      none      >] __slab_alloc+0x68/0xc0 mm/slub.c:2505
[<     inline     >] slab_alloc_node mm/slub.c:2568
[<     inline     >] slab_alloc mm/slub.c:2610
[<      none      >] kmem_cache_alloc_trace+0x263/0x3d0 mm/slub.c:2627
[<     inline     >] kmalloc include/linux/slab.h:478
[<     inline     >] kzalloc include/linux/slab.h:622
[<      none      >] dummy_hrtimer_create+0x49/0x1a0 sound/drivers/dummy.c:464
[<      none      >] dummy_pcm_open+0xcd/0x5e0 sound/drivers/dummy.c:574
[<      none      >] snd_pcm_open_substream+0x188/0x430
sound/core/pcm_native.c:2276
[<     inline     >] snd_pcm_oss_open_file sound/core/oss/pcm_oss.c:2346
[<      none      >] snd_pcm_oss_open.part.17+0x5a4/0x1110
sound/core/oss/pcm_oss.c:2428
[<      none      >] snd_pcm_oss_open+0x35/0x50 sound/core/oss/pcm_oss.c:2392
[<      none      >] soundcore_open+0x30f/0x640 sound/sound_core.c:639
[<      none      >] chrdev_open+0x22a/0x4c0 fs/char_dev.c:388
[<      none      >] do_dentry_open+0x6a2/0xcb0 fs/open.c:736
[<      none      >] vfs_open+0x17b/0x1f0 fs/open.c:853
[<     inline     >] do_last fs/namei.c:3238
[<      none      >] path_openat+0x51bb/0x5ce0 fs/namei.c:3374
[<      none      >] do_filp_open+0x18e/0x250 fs/namei.c:3409
[<      none      >] do_sys_open+0x1fc/0x420 fs/open.c:1020

INFO: Freed in 0xfffd8e09 age=18446705496313138713 cpu=2164287125 pid=-1
[<      none      >] dummy_hrtimer_free+0x68/0x80 sound/drivers/dummy.c:481
[<      none      >] __slab_free+0x1e8/0x300 mm/slub.c:2687
[<     inline     >] slab_free mm/slub.c:2840
[<      none      >] kfree+0x2fc/0x370 mm/slub.c:3691
[<      none      >] dummy_hrtimer_free+0x68/0x80 sound/drivers/dummy.c:481
[<      none      >] dummy_pcm_close+0x9c/0xd0 sound/drivers/dummy.c:607
[<      none      >] snd_pcm_release_substream.part.37+0x169/0x2f0
sound/core/pcm_native.c:2241
[<      none      >] snd_pcm_release_substream+0x59/0x70
sound/core/pcm_native.c:2251
[<      none      >] snd_pcm_oss_release_file+0x7b/0xb0
sound/core/oss/pcm_oss.c:2305
[<      none      >] snd_pcm_oss_release+0xfa/0x280
sound/core/oss/pcm_oss.c:2485
[<      none      >] __fput+0x236/0x780 fs/file_table.c:208
[<      none      >] ____fput+0x15/0x20 fs/file_table.c:244
[<      none      >] task_work_run+0x170/0x210 kernel/task_work.c:115
[<     inline     >] exit_task_work include/linux/task_work.h:21
[<      none      >] do_exit+0x874/0x2d80 kernel/exit.c:748
[<      none      >] do_group_exit+0x108/0x330 kernel/exit.c:878
[<      none      >] get_signal+0x634/0x15e0 kernel/signal.c:2307
[<      none      >] do_signal+0x7f/0x1cf0 arch/x86/kernel/signal.c:784

INFO: Indirect in 0xfffd8e17 age=4294807066 cpu=0 pid=0
INFO: Slab 0xffffea0001796d00 objects=24 used=15 fp=0xffff88005e5b71e8
flags=0x4fffe0000004080
INFO: Object 0xffff88005e5b6f40 @offset=12096 fp=0xbbbbbbbbbbbbbbbb
CPU: 1 PID: 8984 Comm: syz-executor Tainted: G    B           4.6.0-rc6+ #355
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 ffffffff87eb25c0 ffff880059fd7378 ffffffff82c9062f ffffffff5e5b4000
 fffffbfff0fd64b8 ffff88003e804d40 ffff88005e5b6f40 ffffea0001796d00
 ffff88005e5b4000 0000000000000000 ffff880059fd73a8 ffffffff81793e1d

Call Trace:
 [<ffffffff8179e59e>] __asan_report_load8_noabort+0x3e/0x40
mm/kasan/report.c:333
 [<     inline     >] rb_set_parent include/linux/rbtree_augmented.h:111
 [<     inline     >] __rb_erase_augmented include/linux/rbtree_augmented.h:218
 [<ffffffff82ca5787>] rb_erase+0x1b17/0x2010 lib/rbtree.c:427
 [<ffffffff82cb02e8>] timerqueue_del+0x78/0x170 lib/timerqueue.c:86
 [<ffffffff814d0c80>] __remove_hrtimer+0x90/0x220 kernel/time/hrtimer.c:903
 [<     inline     >] remove_hrtimer kernel/time/hrtimer.c:945
 [<ffffffff814d23da>] hrtimer_try_to_cancel+0x22a/0x570
kernel/time/hrtimer.c:1046
 [<ffffffff814d2742>] hrtimer_cancel+0x22/0x40 kernel/time/hrtimer.c:1066
 [<ffffffff85420531>] dummy_hrtimer_stop+0x91/0xb0 sound/drivers/dummy.c:417
 [<ffffffff854228bf>] dummy_pcm_trigger+0x17f/0x1e0 sound/drivers/dummy.c:507
 [<ffffffff85392170>] snd_pcm_do_stop+0x160/0x1b0 sound/core/pcm_native.c:1106
 [<ffffffff85391b26>] snd_pcm_action_single+0x76/0x120
sound/core/pcm_native.c:956
 [<ffffffff85391e01>] snd_pcm_action+0x231/0x290 sound/core/pcm_native.c:974
 [<     inline     >] snd_pcm_stop sound/core/pcm_native.c:1139
 [<ffffffff8539754d>] snd_pcm_drop+0x12d/0x1d0 sound/core/pcm_native.c:1784
 [<ffffffff8539d3be>] snd_pcm_common_ioctl1+0xfae/0x2150
sound/core/pcm_native.c:2805
 [<ffffffff8539ee91>] snd_pcm_capture_ioctl1+0x2a1/0x5e0
sound/core/pcm_native.c:2976
 [<ffffffff8539f2ec>] snd_pcm_kernel_ioctl+0x11c/0x160
sound/core/pcm_native.c:3020
 [<ffffffff853d9a44>] snd_pcm_oss_sync+0x3a4/0xa30 sound/core/oss/pcm_oss.c:1693
 [<ffffffff853da27d>] snd_pcm_oss_release+0x1ad/0x280
sound/core/oss/pcm_oss.c:2483
 [<ffffffff817fc066>] __fput+0x236/0x780 fs/file_table.c:208
 [<ffffffff817fc635>] ____fput+0x15/0x20 fs/file_table.c:244
 [<ffffffff813c91d0>] task_work_run+0x170/0x210 kernel/task_work.c:115
 [<     inline     >] exit_task_work include/linux/task_work.h:21
 [<ffffffff81373e64>] do_exit+0x874/0x2d80 kernel/exit.c:748
 [<ffffffff813764e8>] do_group_exit+0x108/0x330 kernel/exit.c:878
 [<ffffffff81399674>] get_signal+0x634/0x15e0 kernel/signal.c:2307
 [<ffffffff811f40af>] do_signal+0x7f/0x1cf0 arch/x86/kernel/signal.c:784
 [<ffffffff81006695>] exit_to_usermode_loop+0x1a5/0x210
arch/x86/entry/common.c:229
 [<     inline     >] prepare_exit_to_usermode arch/x86/entry/common.c:264
 [<ffffffff8100868f>] syscall_return_slowpath+0x2bf/0x340
arch/x86/entry/common.c:329
 [<ffffffff867c56dc>] entry_SYSCALL_64_fastpath+0xbf/0xc1
arch/x86/entry/entry_64.S:241
==================================================================





==================================================================
BUG: KASAN: use-after-free in timerqueue_add+0x29a/0x2a0 at addr
ffff880062134580
Read of size 8 by task a.out/12711
=============================================================================
BUG kmalloc-192 (Not tainted): kasan: bad access detected
-----------------------------------------------------------------------------

Disabling lock debugging due to kernel taint
INFO: Allocated in 0xbbbbbbbbbbbbbbbb age=18446689703718641197
cpu=2172593693 pid=-1
[<     inline     >] kmalloc include/linux/slab.h:478
[<     inline     >] kzalloc include/linux/slab.h:622
[<      none      >] dummy_hrtimer_create+0x49/0x1a0 sound/drivers/dummy.c:464
[<      none      >] ___slab_alloc+0x55d/0x5a0 mm/slub.c:2476
[<      none      >] __slab_alloc+0x68/0xc0 mm/slub.c:2505
[<     inline     >] slab_alloc_node mm/slub.c:2568
[<     inline     >] slab_alloc mm/slub.c:2610
[<      none      >] kmem_cache_alloc_trace+0x263/0x3d0 mm/slub.c:2627
[<     inline     >] kmalloc include/linux/slab.h:478
[<     inline     >] kzalloc include/linux/slab.h:622
[<      none      >] dummy_hrtimer_create+0x49/0x1a0 sound/drivers/dummy.c:464
[<      none      >] dummy_pcm_open+0xcd/0x5e0 sound/drivers/dummy.c:574
[<      none      >] snd_pcm_open_substream+0x188/0x430
sound/core/pcm_native.c:2276
[<     inline     >] snd_pcm_oss_open_file sound/core/oss/pcm_oss.c:2346
[<      none      >] snd_pcm_oss_open.part.17+0x5a4/0x1110
sound/core/oss/pcm_oss.c:2428
[<      none      >] snd_pcm_oss_open+0x35/0x50 sound/core/oss/pcm_oss.c:2392
[<      none      >] soundcore_open+0x30f/0x640 sound/sound_core.c:639
[<      none      >] chrdev_open+0x22a/0x4c0 fs/char_dev.c:388
[<      none      >] do_dentry_open+0x6a2/0xcb0 fs/open.c:736
[<      none      >] vfs_open+0x17b/0x1f0 fs/open.c:853
[<     inline     >] do_last fs/namei.c:3238
[<      none      >] path_openat+0x51bb/0x5ce0 fs/namei.c:3374
[<      none      >] do_filp_open+0x18e/0x250 fs/namei.c:3409
[<      none      >] do_sys_open+0x1fc/0x420 fs/open.c:1020

INFO: Freed in 0x100015e1b age=18446689836862627373 cpu=0 pid=0
[<      none      >] dummy_hrtimer_free+0x68/0x80 sound/drivers/dummy.c:481
[<      none      >] __slab_free+0x1e8/0x300 mm/slub.c:2687
[<     inline     >] slab_free mm/slub.c:2840
[<      none      >] kfree+0x2fc/0x370 mm/slub.c:3691
[<      none      >] dummy_hrtimer_free+0x68/0x80 sound/drivers/dummy.c:481
[<      none      >] dummy_pcm_close+0x9c/0xd0 sound/drivers/dummy.c:607
[<      none      >] snd_pcm_release_substream.part.37+0x169/0x2f0
sound/core/pcm_native.c:2241
[<      none      >] snd_pcm_release_substream+0x59/0x70
sound/core/pcm_native.c:2251
[<      none      >] snd_pcm_oss_release_file+0x7b/0xb0
sound/core/oss/pcm_oss.c:2305
[<      none      >] snd_pcm_oss_release+0xfa/0x280
sound/core/oss/pcm_oss.c:2485
[<      none      >] __fput+0x236/0x780 fs/file_table.c:208
[<      none      >] ____fput+0x15/0x20 fs/file_table.c:244
[<      none      >] task_work_run+0x170/0x210 kernel/task_work.c:115
[<     inline     >] exit_task_work include/linux/task_work.h:21
[<      none      >] do_exit+0x874/0x2d80 kernel/exit.c:748
[<      none      >] do_group_exit+0x108/0x330 kernel/exit.c:878
[<     inline     >] SYSC_exit_group kernel/exit.c:889
[<      none      >] SyS_exit_group+0x1d/0x20 kernel/exit.c:887
[<      none      >] entry_SYSCALL_64_fastpath+0x23/0xc1
arch/x86/entry/entry_64.S:207

INFO: Indirect in 0x100015e2e age=4295056943 cpu=0 pid=0
INFO: Slab 0xffffea0001884d00 objects=24 used=10 fp=0xffff880062135a48
flags=0x4fffe0000004080
INFO: Object 0xffff880062134540 @offset=1344 fp=0xbbbbbbbbbbbbbbbb
CPU: 2 PID: 12711 Comm: a.out Tainted: G    B           4.6.0-rc6+ #355
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 ffffffff87eb25c0 ffff88006d407c60 ffffffff82c9062f ffffffff62134000
 fffffbfff0fd64b8 ffff88003e804d40 ffff880062134540 ffffea0001884d00
 ffff880062134000 ffff88006d417f10 ffff88006d407c90 ffffffff81793e1d

Call Trace:
 [<ffffffff8179e59e>] __asan_report_load8_noabort+0x3e/0x40
mm/kasan/report.c:333
 [<ffffffff82cb022a>] timerqueue_add+0x29a/0x2a0 lib/timerqueue.c:51
 [<ffffffff814d1066>] enqueue_hrtimer+0x116/0x3d0 kernel/time/hrtimer.c:879
 [<     inline     >] __run_hrtimer kernel/time/hrtimer.c:1257
 [<ffffffff814d1c5e>] __hrtimer_run_queues+0x93e/0xe90
kernel/time/hrtimer.c:1306
 [<ffffffff814d4022>] hrtimer_interrupt+0x182/0x430 kernel/time/hrtimer.c:1340
 [<ffffffff8125aa62>] local_apic_timer_interrupt+0x72/0xe0
arch/x86/kernel/apic/apic.c:907
 [<ffffffff867c7f59>] smp_apic_timer_interrupt+0x79/0xa0
arch/x86/kernel/apic/apic.c:931
 [<ffffffff867c62ac>] apic_timer_interrupt+0x8c/0xa0
arch/x86/entry/entry_64.S:454
 [<ffffffff813c93d3>] __kernel_text_address+0x73/0xa0 kernel/extable.c:103
 [<ffffffff811fd73e>] print_context_stack+0x6e/0xc0
arch/x86/kernel/dumpstack.c:107
 [<ffffffff811fc984>] dump_trace+0x124/0x320 arch/x86/kernel/dumpstack_64.c:243
 [<ffffffff8121d396>] save_stack_trace+0x26/0x50 arch/x86/kernel/stacktrace.c:67
 [<     inline     >] __save_stack_trace mm/kmemleak.c:530
 [<ffffffff817e2467>] create_object+0x137/0x2d0 mm/kmemleak.c:585
 [<ffffffff867a7d43>] kmemleak_alloc+0x63/0xa0 mm/kmemleak.c:915
 [<     inline     >] kmemleak_alloc_recursive include/linux/kmemleak.h:47
 [<     inline     >] slab_post_alloc_hook mm/slab.h:406
 [<     inline     >] slab_alloc_node mm/slub.c:2602
 [<     inline     >] slab_alloc mm/slub.c:2610
 [<ffffffff81798236>] kmem_cache_alloc+0x166/0x3c0 mm/slub.c:2615
 [<ffffffff81727df0>] ptlock_alloc+0x20/0x80 mm/memory.c:3968
 [<     inline     >] ptlock_init include/linux/mm.h:1625
 [<     inline     >] pgtable_page_ctor include/linux/mm.h:1659
 [<ffffffff81299ba9>] pte_alloc_one+0x59/0x100 arch/x86/mm/pgtable.c:31
 [<ffffffff81716c68>] __pte_alloc+0x28/0x2a0 mm/memory.c:569
 [<     inline     >] __handle_mm_fault mm/memory.c:3470
 [<ffffffff81721a6a>] handle_mm_fault+0xc0a/0x11a0 mm/memory.c:3522
 [<ffffffff8128ad47>] __do_page_fault+0x457/0xbb0 arch/x86/mm/fault.c:1351
 [<ffffffff8128b5ef>] trace_do_page_fault+0xdf/0x5b0 arch/x86/mm/fault.c:1444
 [<ffffffff8127bd84>] do_async_page_fault+0x14/0xd0 arch/x86/kernel/kvm.c:265
 [<ffffffff867c77f8>] async_page_fault+0x28/0x30 arch/x86/entry/entry_64.S:920
==================================================================


On commit 83858a701cf3271f81dd321c2a81e5666c6ca8f4 (note it is not
super fresh: May 3).

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: sound: use-after-free in hrtimer_cancel
  2016-06-04 18:00 sound: use-after-free in hrtimer_cancel Dmitry Vyukov
@ 2016-06-04 18:27 ` Dmitry Vyukov
  2016-06-06 14:11   ` Takashi Iwai
  0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Vyukov @ 2016-06-04 18:27 UTC (permalink / raw)
  To: Takashi Iwai, Jaroslav Kysela, alsa-devel, LKML
  Cc: Sasha Levin, Alexander Potapenko, Kostya Serebryany, syzkaller

On Sat, Jun 4, 2016 at 8:00 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> Hello,
>
> The following program triggers use-after-free:

Forget to mention that you need to run it in a tight parallel loop. It
takes around 5 minutes to reproduce for me.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: sound: use-after-free in hrtimer_cancel
  2016-06-04 18:27 ` Dmitry Vyukov
@ 2016-06-06 14:11   ` Takashi Iwai
  2016-06-06 16:29     ` Dmitry Vyukov
  2016-06-21 18:26     ` Dmitry Vyukov
  0 siblings, 2 replies; 13+ messages in thread
From: Takashi Iwai @ 2016-06-06 14:11 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Jaroslav Kysela, alsa-devel, LKML, Sasha Levin,
	Alexander Potapenko, Kostya Serebryany, syzkaller

On Sat, 04 Jun 2016 20:27:50 +0200,
Dmitry Vyukov wrote:
> 
> On Sat, Jun 4, 2016 at 8:00 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> > Hello,
> >
> > The following program triggers use-after-free:
> 
> Forget to mention that you need to run it in a tight parallel loop. It
> takes around 5 minutes to reproduce for me.

Hmm, this again is a bug that is difficult to trigger...  At least, I
couldn't reproduce locally.  How many processes are you running with
stress program?

It seems that there is nothing more than opening /dev/audio and does
some mmap in the job.  Is there any other relevant thing there?

Also, this assumes that the first sound card is Dummy driver, right?
Check /proc/asound/cards.

If it's about snd-dummy driver, one blind shot would be a patch like
below.  But even if it would fix, it doesn't explain why it's
triggered in that way...


thanks,

Takashi

---
diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c
index c0f8f613f1f1..172dacd925f5 100644
--- a/sound/drivers/dummy.c
+++ b/sound/drivers/dummy.c
@@ -420,6 +420,7 @@ static int dummy_hrtimer_stop(struct snd_pcm_substream *substream)
 
 static inline void dummy_hrtimer_sync(struct dummy_hrtimer_pcm *dpcm)
 {
+	hrtimer_cancel(&dpcm->timer);
 	tasklet_kill(&dpcm->tasklet);
 }
 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: sound: use-after-free in hrtimer_cancel
  2016-06-06 14:11   ` Takashi Iwai
@ 2016-06-06 16:29     ` Dmitry Vyukov
  2016-06-06 16:39       ` Takashi Iwai
  2016-06-21 18:26     ` Dmitry Vyukov
  1 sibling, 1 reply; 13+ messages in thread
From: Dmitry Vyukov @ 2016-06-06 16:29 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Jaroslav Kysela, alsa-devel, LKML, Sasha Levin,
	Alexander Potapenko, Kostya Serebryany, syzkaller

On Mon, Jun 6, 2016 at 4:11 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Sat, 04 Jun 2016 20:27:50 +0200,
> Dmitry Vyukov wrote:
>>
>> On Sat, Jun 4, 2016 at 8:00 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> > Hello,
>> >
>> > The following program triggers use-after-free:
>>
>> Forget to mention that you need to run it in a tight parallel loop. It
>> takes around 5 minutes to reproduce for me.
>
> Hmm, this again is a bug that is difficult to trigger...  At least, I
> couldn't reproduce locally.  How many processes are you running with
> stress program?

I use a VM with 4 cores and use 20 parallel test processes.

> It seems that there is nothing more than opening /dev/audio and does
> some mmap in the job.  Is there any other relevant thing there?


I think poll with timeout is related. It is poll who sets hrtimer, right?


> Also, this assumes that the first sound card is Dummy driver, right?
> Check /proc/asound/cards.

# cat /proc/asound/cards
 0 [Dummy          ]: Dummy - Dummy
                      Dummy 1
 1 [Loopback       ]: Loopback - Loopback
                      Loopback 1
 2 [VirMIDI        ]: VirMIDI - VirMIDI
                      Virtual MIDI Card 1
 3 [pcsp           ]: PC-Speaker - pcsp
                      Internal PC-Speaker at port 0x61
 4 [Intel          ]: HDA-Intel - HDA Intel
                      HDA Intel at 0xfebf0000 irq 24



> If it's about snd-dummy driver, one blind shot would be a patch like
> below.  But even if it would fix, it doesn't explain why it's
> triggered in that way...
>
>
> thanks,
>
> Takashi
>
> ---
> diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c
> index c0f8f613f1f1..172dacd925f5 100644
> --- a/sound/drivers/dummy.c
> +++ b/sound/drivers/dummy.c
> @@ -420,6 +420,7 @@ static int dummy_hrtimer_stop(struct snd_pcm_substream *substream)
>
>  static inline void dummy_hrtimer_sync(struct dummy_hrtimer_pcm *dpcm)
>  {
> +       hrtimer_cancel(&dpcm->timer);
>         tasklet_kill(&dpcm->tasklet);
>  }
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: sound: use-after-free in hrtimer_cancel
  2016-06-06 16:29     ` Dmitry Vyukov
@ 2016-06-06 16:39       ` Takashi Iwai
  2016-06-21 17:41         ` Dmitry Vyukov
  0 siblings, 1 reply; 13+ messages in thread
From: Takashi Iwai @ 2016-06-06 16:39 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Jaroslav Kysela, alsa-devel, LKML, Sasha Levin,
	Alexander Potapenko, Kostya Serebryany, syzkaller

On Mon, 06 Jun 2016 18:29:25 +0200,
Dmitry Vyukov wrote:
> 
> On Mon, Jun 6, 2016 at 4:11 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > On Sat, 04 Jun 2016 20:27:50 +0200,
> > Dmitry Vyukov wrote:
> >>
> >> On Sat, Jun 4, 2016 at 8:00 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> >> > Hello,
> >> >
> >> > The following program triggers use-after-free:
> >>
> >> Forget to mention that you need to run it in a tight parallel loop. It
> >> takes around 5 minutes to reproduce for me.
> >
> > Hmm, this again is a bug that is difficult to trigger...  At least, I
> > couldn't reproduce locally.  How many processes are you running with
> > stress program?
> 
> I use a VM with 4 cores and use 20 parallel test processes.
> 
> > It seems that there is nothing more than opening /dev/audio and does
> > some mmap in the job.  Is there any other relevant thing there?
> 
> 
> I think poll with timeout is related. It is poll who sets hrtimer, right?

If it's about snd-dummy driver, hrtimer is created at open, and
started/stopped at PCM trigger, and removed at close.

Is there any good way to decode which syscalls are executed in the
test code?


Takashi

> > Also, this assumes that the first sound card is Dummy driver, right?
> > Check /proc/asound/cards.
> 
> # cat /proc/asound/cards
>  0 [Dummy          ]: Dummy - Dummy
>                       Dummy 1
>  1 [Loopback       ]: Loopback - Loopback
>                       Loopback 1
>  2 [VirMIDI        ]: VirMIDI - VirMIDI
>                       Virtual MIDI Card 1
>  3 [pcsp           ]: PC-Speaker - pcsp
>                       Internal PC-Speaker at port 0x61
>  4 [Intel          ]: HDA-Intel - HDA Intel
>                       HDA Intel at 0xfebf0000 irq 24
> 
> 
> 
> > If it's about snd-dummy driver, one blind shot would be a patch like
> > below.  But even if it would fix, it doesn't explain why it's
> > triggered in that way...
> >
> >
> > thanks,
> >
> > Takashi
> >
> > ---
> > diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c
> > index c0f8f613f1f1..172dacd925f5 100644
> > --- a/sound/drivers/dummy.c
> > +++ b/sound/drivers/dummy.c
> > @@ -420,6 +420,7 @@ static int dummy_hrtimer_stop(struct snd_pcm_substream *substream)
> >
> >  static inline void dummy_hrtimer_sync(struct dummy_hrtimer_pcm *dpcm)
> >  {
> > +       hrtimer_cancel(&dpcm->timer);
> >         tasklet_kill(&dpcm->tasklet);
> >  }
> >
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: sound: use-after-free in hrtimer_cancel
  2016-06-06 16:39       ` Takashi Iwai
@ 2016-06-21 17:41         ` Dmitry Vyukov
  2016-06-24 13:33           ` Takashi Iwai
  0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Vyukov @ 2016-06-21 17:41 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Jaroslav Kysela, alsa-devel, LKML, Sasha Levin,
	Alexander Potapenko, Kostya Serebryany, syzkaller

On Mon, Jun 6, 2016 at 6:39 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Mon, 06 Jun 2016 18:29:25 +0200,
> Dmitry Vyukov wrote:
>>
>> On Mon, Jun 6, 2016 at 4:11 PM, Takashi Iwai <tiwai@suse.de> wrote:
>> > On Sat, 04 Jun 2016 20:27:50 +0200,
>> > Dmitry Vyukov wrote:
>> >>
>> >> On Sat, Jun 4, 2016 at 8:00 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> >> > Hello,
>> >> >
>> >> > The following program triggers use-after-free:
>> >>
>> >> Forget to mention that you need to run it in a tight parallel loop. It
>> >> takes around 5 minutes to reproduce for me.
>> >
>> > Hmm, this again is a bug that is difficult to trigger...  At least, I
>> > couldn't reproduce locally.  How many processes are you running with
>> > stress program?
>>
>> I use a VM with 4 cores and use 20 parallel test processes.
>>
>> > It seems that there is nothing more than opening /dev/audio and does
>> > some mmap in the job.  Is there any other relevant thing there?
>>
>>
>> I think poll with timeout is related. It is poll who sets hrtimer, right?
>
> If it's about snd-dummy driver, hrtimer is created at open, and
> started/stopped at PCM trigger, and removed at close.
>
> Is there any good way to decode which syscalls are executed in the
> test code?

What do you mean?
Here are the syscalls in the program:

    r[2] = syscall(SYS_open, "/dev/audio", 0xa40ul, 0, 0, 0);
    // r[2] is in the descriptor passed to SYS_poll
    r[15] = syscall(SYS_poll, 0x2001dde8ul, 0x4ul, 0x8ul, 0, 0, 0);
    r[18] = syscall(SYS_readv, r[2], 0x20dc13c0ul, 0x1ul, 0, 0, 0);
    r[19] = syscall(SYS_read, r[2], 0x20dbefe0ul, 0x20ul, 0, 0, 0);

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: sound: use-after-free in hrtimer_cancel
  2016-06-06 14:11   ` Takashi Iwai
  2016-06-06 16:29     ` Dmitry Vyukov
@ 2016-06-21 18:26     ` Dmitry Vyukov
  2016-06-24 13:32       ` Takashi Iwai
  1 sibling, 1 reply; 13+ messages in thread
From: Dmitry Vyukov @ 2016-06-21 18:26 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Jaroslav Kysela, alsa-devel, LKML, Sasha Levin,
	Alexander Potapenko, Kostya Serebryany, syzkaller

On Mon, Jun 6, 2016 at 4:11 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Sat, 04 Jun 2016 20:27:50 +0200,
> Dmitry Vyukov wrote:
>>
>> On Sat, Jun 4, 2016 at 8:00 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> > Hello,
>> >
>> > The following program triggers use-after-free:
>>
>> Forget to mention that you need to run it in a tight parallel loop. It
>> takes around 5 minutes to reproduce for me.
>
> Hmm, this again is a bug that is difficult to trigger...  At least, I
> couldn't reproduce locally.  How many processes are you running with
> stress program?
>
> It seems that there is nothing more than opening /dev/audio and does
> some mmap in the job.  Is there any other relevant thing there?
>
> Also, this assumes that the first sound card is Dummy driver, right?
> Check /proc/asound/cards.
>
> If it's about snd-dummy driver, one blind shot would be a patch like
> below.  But even if it would fix, it doesn't explain why it's
> triggered in that way...
>
>
> thanks,
>
> Takashi
>
> ---
> diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c
> index c0f8f613f1f1..172dacd925f5 100644
> --- a/sound/drivers/dummy.c
> +++ b/sound/drivers/dummy.c
> @@ -420,6 +420,7 @@ static int dummy_hrtimer_stop(struct snd_pcm_substream *substream)
>
>  static inline void dummy_hrtimer_sync(struct dummy_hrtimer_pcm *dpcm)
>  {
> +       hrtimer_cancel(&dpcm->timer);
>         tasklet_kill(&dpcm->tasklet);
>  }


Yes, this seems to fix it. I've stressed it for an hour with several
reproducers. If I am not mistaken, it also makes test cases run 15%
faster.
Please mail a patch.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: sound: use-after-free in hrtimer_cancel
  2016-06-21 18:26     ` Dmitry Vyukov
@ 2016-06-24 13:32       ` Takashi Iwai
  2016-06-24 13:33         ` Dmitry Vyukov
  0 siblings, 1 reply; 13+ messages in thread
From: Takashi Iwai @ 2016-06-24 13:32 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Jaroslav Kysela, alsa-devel, LKML, Sasha Levin,
	Alexander Potapenko, Kostya Serebryany, syzkaller

On Tue, 21 Jun 2016 20:26:48 +0200,
Dmitry Vyukov wrote:
> 
> On Mon, Jun 6, 2016 at 4:11 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > On Sat, 04 Jun 2016 20:27:50 +0200,
> > Dmitry Vyukov wrote:
> >>
> >> On Sat, Jun 4, 2016 at 8:00 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> >> > Hello,
> >> >
> >> > The following program triggers use-after-free:
> >>
> >> Forget to mention that you need to run it in a tight parallel loop. It
> >> takes around 5 minutes to reproduce for me.
> >
> > Hmm, this again is a bug that is difficult to trigger...  At least, I
> > couldn't reproduce locally.  How many processes are you running with
> > stress program?
> >
> > It seems that there is nothing more than opening /dev/audio and does
> > some mmap in the job.  Is there any other relevant thing there?
> >
> > Also, this assumes that the first sound card is Dummy driver, right?
> > Check /proc/asound/cards.
> >
> > If it's about snd-dummy driver, one blind shot would be a patch like
> > below.  But even if it would fix, it doesn't explain why it's
> > triggered in that way...
> >
> >
> > thanks,
> >
> > Takashi
> >
> > ---
> > diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c
> > index c0f8f613f1f1..172dacd925f5 100644
> > --- a/sound/drivers/dummy.c
> > +++ b/sound/drivers/dummy.c
> > @@ -420,6 +420,7 @@ static int dummy_hrtimer_stop(struct snd_pcm_substream *substream)
> >
> >  static inline void dummy_hrtimer_sync(struct dummy_hrtimer_pcm *dpcm)
> >  {
> > +       hrtimer_cancel(&dpcm->timer);
> >         tasklet_kill(&dpcm->tasklet);
> >  }
> 
> 
> Yes, this seems to fix it. I've stressed it for an hour with several
> reproducers. If I am not mistaken, it also makes test cases run 15%
> faster.

Interesting.  Possibly because it now syncs properly before other
restart of stream or such...

> Please mail a patch.

OK, below is the formal patch.
Let me know if I can give your tested-by tag.


thanks,

Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: dummy: Fix a use-after-free at closing

syzkaller fuzzer spotted a potential use-after-free case in snd-dummy
driver when hrtimer is used as backend:
> ==================================================================
> BUG: KASAN: use-after-free in rb_erase+0x1b17/0x2010 at addr ffff88005e5b6f68
>  Read of size 8 by task syz-executor/8984
> =============================================================================
> BUG kmalloc-192 (Not tainted): kasan: bad access detected
> -----------------------------------------------------------------------------
>
> Disabling lock debugging due to kernel taint
> INFO: Allocated in 0xbbbbbbbbbbbbbbbb age=18446705582212484632
> ....
> [<      none      >] dummy_hrtimer_create+0x49/0x1a0 sound/drivers/dummy.c:464
> ....
> INFO: Freed in 0xfffd8e09 age=18446705496313138713 cpu=2164287125 pid=-1
> [<      none      >] dummy_hrtimer_free+0x68/0x80 sound/drivers/dummy.c:481
> ....
> Call Trace:
>  [<ffffffff8179e59e>] __asan_report_load8_noabort+0x3e/0x40 mm/kasan/report.c:333
>  [<     inline     >] rb_set_parent include/linux/rbtree_augmented.h:111
>  [<     inline     >] __rb_erase_augmented include/linux/rbtree_augmented.h:218
>  [<ffffffff82ca5787>] rb_erase+0x1b17/0x2010 lib/rbtree.c:427
>  [<ffffffff82cb02e8>] timerqueue_del+0x78/0x170 lib/timerqueue.c:86
>  [<ffffffff814d0c80>] __remove_hrtimer+0x90/0x220 kernel/time/hrtimer.c:903
>  [<     inline     >] remove_hrtimer kernel/time/hrtimer.c:945
>  [<ffffffff814d23da>] hrtimer_try_to_cancel+0x22a/0x570 kernel/time/hrtimer.c:1046
>  [<ffffffff814d2742>] hrtimer_cancel+0x22/0x40 kernel/time/hrtimer.c:1066
>  [<ffffffff85420531>] dummy_hrtimer_stop+0x91/0xb0 sound/drivers/dummy.c:417
>  [<ffffffff854228bf>] dummy_pcm_trigger+0x17f/0x1e0 sound/drivers/dummy.c:507
>  [<ffffffff85392170>] snd_pcm_do_stop+0x160/0x1b0 sound/core/pcm_native.c:1106
>  [<ffffffff85391b26>] snd_pcm_action_single+0x76/0x120 sound/core/pcm_native.c:956
>  [<ffffffff85391e01>] snd_pcm_action+0x231/0x290 sound/core/pcm_native.c:974
>  [<     inline     >] snd_pcm_stop sound/core/pcm_native.c:1139
>  [<ffffffff8539754d>] snd_pcm_drop+0x12d/0x1d0 sound/core/pcm_native.c:1784
>  [<ffffffff8539d3be>] snd_pcm_common_ioctl1+0xfae/0x2150 sound/core/pcm_native.c:2805
>  [<ffffffff8539ee91>] snd_pcm_capture_ioctl1+0x2a1/0x5e0 sound/core/pcm_native.c:2976
>  [<ffffffff8539f2ec>] snd_pcm_kernel_ioctl+0x11c/0x160 sound/core/pcm_native.c:3020
>  [<ffffffff853d9a44>] snd_pcm_oss_sync+0x3a4/0xa30 sound/core/oss/pcm_oss.c:1693
>  [<ffffffff853da27d>] snd_pcm_oss_release+0x1ad/0x280 sound/core/oss/pcm_oss.c:2483
>  .....

A workaround is to call hrtimer_cancel() in dummy_hrtimer_sync() which
is called certainly before other blocking ops.

Reported-by: Dmitry Vyukov <dvyukov@google.com>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/drivers/dummy.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c
index c0f8f613f1f1..172dacd925f5 100644
--- a/sound/drivers/dummy.c
+++ b/sound/drivers/dummy.c
@@ -420,6 +420,7 @@ static int dummy_hrtimer_stop(struct snd_pcm_substream *substream)
 
 static inline void dummy_hrtimer_sync(struct dummy_hrtimer_pcm *dpcm)
 {
+	hrtimer_cancel(&dpcm->timer);
 	tasklet_kill(&dpcm->tasklet);
 }
 
-- 
2.9.0

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: sound: use-after-free in hrtimer_cancel
  2016-06-24 13:32       ` Takashi Iwai
@ 2016-06-24 13:33         ` Dmitry Vyukov
  2016-06-24 14:37           ` Takashi Iwai
  0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Vyukov @ 2016-06-24 13:33 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Jaroslav Kysela, alsa-devel, LKML, Sasha Levin,
	Alexander Potapenko, Kostya Serebryany, syzkaller

On Fri, Jun 24, 2016 at 3:32 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Tue, 21 Jun 2016 20:26:48 +0200,
> Dmitry Vyukov wrote:
>>
>> On Mon, Jun 6, 2016 at 4:11 PM, Takashi Iwai <tiwai@suse.de> wrote:
>> > On Sat, 04 Jun 2016 20:27:50 +0200,
>> > Dmitry Vyukov wrote:
>> >>
>> >> On Sat, Jun 4, 2016 at 8:00 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> >> > Hello,
>> >> >
>> >> > The following program triggers use-after-free:
>> >>
>> >> Forget to mention that you need to run it in a tight parallel loop. It
>> >> takes around 5 minutes to reproduce for me.
>> >
>> > Hmm, this again is a bug that is difficult to trigger...  At least, I
>> > couldn't reproduce locally.  How many processes are you running with
>> > stress program?
>> >
>> > It seems that there is nothing more than opening /dev/audio and does
>> > some mmap in the job.  Is there any other relevant thing there?
>> >
>> > Also, this assumes that the first sound card is Dummy driver, right?
>> > Check /proc/asound/cards.
>> >
>> > If it's about snd-dummy driver, one blind shot would be a patch like
>> > below.  But even if it would fix, it doesn't explain why it's
>> > triggered in that way...
>> >
>> >
>> > thanks,
>> >
>> > Takashi
>> >
>> > ---
>> > diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c
>> > index c0f8f613f1f1..172dacd925f5 100644
>> > --- a/sound/drivers/dummy.c
>> > +++ b/sound/drivers/dummy.c
>> > @@ -420,6 +420,7 @@ static int dummy_hrtimer_stop(struct snd_pcm_substream *substream)
>> >
>> >  static inline void dummy_hrtimer_sync(struct dummy_hrtimer_pcm *dpcm)
>> >  {
>> > +       hrtimer_cancel(&dpcm->timer);
>> >         tasklet_kill(&dpcm->tasklet);
>> >  }
>>
>>
>> Yes, this seems to fix it. I've stressed it for an hour with several
>> reproducers. If I am not mistaken, it also makes test cases run 15%
>> faster.
>
> Interesting.  Possibly because it now syncs properly before other
> restart of stream or such...
>
>> Please mail a patch.
>
> OK, below is the formal patch.
> Let me know if I can give your tested-by tag.

Sure.

Tested-by: Dmitry Vyukov <dvyukov@google.com>

> -- 8< --
> From: Takashi Iwai <tiwai@suse.de>
> Subject: [PATCH] ALSA: dummy: Fix a use-after-free at closing
>
> syzkaller fuzzer spotted a potential use-after-free case in snd-dummy
> driver when hrtimer is used as backend:
>> ==================================================================
>> BUG: KASAN: use-after-free in rb_erase+0x1b17/0x2010 at addr ffff88005e5b6f68
>>  Read of size 8 by task syz-executor/8984
>> =============================================================================
>> BUG kmalloc-192 (Not tainted): kasan: bad access detected
>> -----------------------------------------------------------------------------
>>
>> Disabling lock debugging due to kernel taint
>> INFO: Allocated in 0xbbbbbbbbbbbbbbbb age=18446705582212484632
>> ....
>> [<      none      >] dummy_hrtimer_create+0x49/0x1a0 sound/drivers/dummy.c:464
>> ....
>> INFO: Freed in 0xfffd8e09 age=18446705496313138713 cpu=2164287125 pid=-1
>> [<      none      >] dummy_hrtimer_free+0x68/0x80 sound/drivers/dummy.c:481
>> ....
>> Call Trace:
>>  [<ffffffff8179e59e>] __asan_report_load8_noabort+0x3e/0x40 mm/kasan/report.c:333
>>  [<     inline     >] rb_set_parent include/linux/rbtree_augmented.h:111
>>  [<     inline     >] __rb_erase_augmented include/linux/rbtree_augmented.h:218
>>  [<ffffffff82ca5787>] rb_erase+0x1b17/0x2010 lib/rbtree.c:427
>>  [<ffffffff82cb02e8>] timerqueue_del+0x78/0x170 lib/timerqueue.c:86
>>  [<ffffffff814d0c80>] __remove_hrtimer+0x90/0x220 kernel/time/hrtimer.c:903
>>  [<     inline     >] remove_hrtimer kernel/time/hrtimer.c:945
>>  [<ffffffff814d23da>] hrtimer_try_to_cancel+0x22a/0x570 kernel/time/hrtimer.c:1046
>>  [<ffffffff814d2742>] hrtimer_cancel+0x22/0x40 kernel/time/hrtimer.c:1066
>>  [<ffffffff85420531>] dummy_hrtimer_stop+0x91/0xb0 sound/drivers/dummy.c:417
>>  [<ffffffff854228bf>] dummy_pcm_trigger+0x17f/0x1e0 sound/drivers/dummy.c:507
>>  [<ffffffff85392170>] snd_pcm_do_stop+0x160/0x1b0 sound/core/pcm_native.c:1106
>>  [<ffffffff85391b26>] snd_pcm_action_single+0x76/0x120 sound/core/pcm_native.c:956
>>  [<ffffffff85391e01>] snd_pcm_action+0x231/0x290 sound/core/pcm_native.c:974
>>  [<     inline     >] snd_pcm_stop sound/core/pcm_native.c:1139
>>  [<ffffffff8539754d>] snd_pcm_drop+0x12d/0x1d0 sound/core/pcm_native.c:1784
>>  [<ffffffff8539d3be>] snd_pcm_common_ioctl1+0xfae/0x2150 sound/core/pcm_native.c:2805
>>  [<ffffffff8539ee91>] snd_pcm_capture_ioctl1+0x2a1/0x5e0 sound/core/pcm_native.c:2976
>>  [<ffffffff8539f2ec>] snd_pcm_kernel_ioctl+0x11c/0x160 sound/core/pcm_native.c:3020
>>  [<ffffffff853d9a44>] snd_pcm_oss_sync+0x3a4/0xa30 sound/core/oss/pcm_oss.c:1693
>>  [<ffffffff853da27d>] snd_pcm_oss_release+0x1ad/0x280 sound/core/oss/pcm_oss.c:2483
>>  .....
>
> A workaround is to call hrtimer_cancel() in dummy_hrtimer_sync() which
> is called certainly before other blocking ops.
>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>  sound/drivers/dummy.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c
> index c0f8f613f1f1..172dacd925f5 100644
> --- a/sound/drivers/dummy.c
> +++ b/sound/drivers/dummy.c
> @@ -420,6 +420,7 @@ static int dummy_hrtimer_stop(struct snd_pcm_substream *substream)
>
>  static inline void dummy_hrtimer_sync(struct dummy_hrtimer_pcm *dpcm)
>  {
> +       hrtimer_cancel(&dpcm->timer);
>         tasklet_kill(&dpcm->tasklet);
>  }
>
> --
> 2.9.0
>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: sound: use-after-free in hrtimer_cancel
  2016-06-21 17:41         ` Dmitry Vyukov
@ 2016-06-24 13:33           ` Takashi Iwai
  2016-06-24 13:48             ` Dmitry Vyukov
  0 siblings, 1 reply; 13+ messages in thread
From: Takashi Iwai @ 2016-06-24 13:33 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Jaroslav Kysela, alsa-devel, LKML, Sasha Levin,
	Alexander Potapenko, Kostya Serebryany, syzkaller

On Tue, 21 Jun 2016 19:41:28 +0200,
Dmitry Vyukov wrote:
> 
> On Mon, Jun 6, 2016 at 6:39 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > On Mon, 06 Jun 2016 18:29:25 +0200,
> > Dmitry Vyukov wrote:
> >>
> >> On Mon, Jun 6, 2016 at 4:11 PM, Takashi Iwai <tiwai@suse.de> wrote:
> >> > On Sat, 04 Jun 2016 20:27:50 +0200,
> >> > Dmitry Vyukov wrote:
> >> >>
> >> >> On Sat, Jun 4, 2016 at 8:00 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> >> >> > Hello,
> >> >> >
> >> >> > The following program triggers use-after-free:
> >> >>
> >> >> Forget to mention that you need to run it in a tight parallel loop. It
> >> >> takes around 5 minutes to reproduce for me.
> >> >
> >> > Hmm, this again is a bug that is difficult to trigger...  At least, I
> >> > couldn't reproduce locally.  How many processes are you running with
> >> > stress program?
> >>
> >> I use a VM with 4 cores and use 20 parallel test processes.
> >>
> >> > It seems that there is nothing more than opening /dev/audio and does
> >> > some mmap in the job.  Is there any other relevant thing there?
> >>
> >>
> >> I think poll with timeout is related. It is poll who sets hrtimer, right?
> >
> > If it's about snd-dummy driver, hrtimer is created at open, and
> > started/stopped at PCM trigger, and removed at close.
> >
> > Is there any good way to decode which syscalls are executed in the
> > test code?
> 
> What do you mean?
> Here are the syscalls in the program:
> 
>     r[2] = syscall(SYS_open, "/dev/audio", 0xa40ul, 0, 0, 0);
>     // r[2] is in the descriptor passed to SYS_poll
>     r[15] = syscall(SYS_poll, 0x2001dde8ul, 0x4ul, 0x8ul, 0, 0, 0);
>     r[18] = syscall(SYS_readv, r[2], 0x20dc13c0ul, 0x1ul, 0, 0, 0);
>     r[19] = syscall(SYS_read, r[2], 0x20dbefe0ul, 0x20ul, 0, 0, 0);

I meant some nice way to decode these magic numbers to be more
understandable :)


Takashi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: sound: use-after-free in hrtimer_cancel
  2016-06-24 13:33           ` Takashi Iwai
@ 2016-06-24 13:48             ` Dmitry Vyukov
  2016-06-24 14:35               ` Takashi Iwai
  0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Vyukov @ 2016-06-24 13:48 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: Jaroslav Kysela, alsa-devel, LKML, Sasha Levin,
	Alexander Potapenko, Kostya Serebryany, syzkaller

On Fri, Jun 24, 2016 at 3:33 PM, Takashi Iwai <tiwai@suse.de> wrote:
> On Tue, 21 Jun 2016 19:41:28 +0200,
> Dmitry Vyukov wrote:
>>
>> On Mon, Jun 6, 2016 at 6:39 PM, Takashi Iwai <tiwai@suse.de> wrote:
>> > On Mon, 06 Jun 2016 18:29:25 +0200,
>> > Dmitry Vyukov wrote:
>> >>
>> >> On Mon, Jun 6, 2016 at 4:11 PM, Takashi Iwai <tiwai@suse.de> wrote:
>> >> > On Sat, 04 Jun 2016 20:27:50 +0200,
>> >> > Dmitry Vyukov wrote:
>> >> >>
>> >> >> On Sat, Jun 4, 2016 at 8:00 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
>> >> >> > Hello,
>> >> >> >
>> >> >> > The following program triggers use-after-free:
>> >> >>
>> >> >> Forget to mention that you need to run it in a tight parallel loop. It
>> >> >> takes around 5 minutes to reproduce for me.
>> >> >
>> >> > Hmm, this again is a bug that is difficult to trigger...  At least, I
>> >> > couldn't reproduce locally.  How many processes are you running with
>> >> > stress program?
>> >>
>> >> I use a VM with 4 cores and use 20 parallel test processes.
>> >>
>> >> > It seems that there is nothing more than opening /dev/audio and does
>> >> > some mmap in the job.  Is there any other relevant thing there?
>> >>
>> >>
>> >> I think poll with timeout is related. It is poll who sets hrtimer, right?
>> >
>> > If it's about snd-dummy driver, hrtimer is created at open, and
>> > started/stopped at PCM trigger, and removed at close.
>> >
>> > Is there any good way to decode which syscalls are executed in the
>> > test code?
>>
>> What do you mean?
>> Here are the syscalls in the program:
>>
>>     r[2] = syscall(SYS_open, "/dev/audio", 0xa40ul, 0, 0, 0);
>>     // r[2] is in the descriptor passed to SYS_poll
>>     r[15] = syscall(SYS_poll, 0x2001dde8ul, 0x4ul, 0x8ul, 0, 0, 0);
>>     r[18] = syscall(SYS_readv, r[2], 0x20dc13c0ul, 0x1ul, 0, 0, 0);
>>     r[19] = syscall(SYS_read, r[2], 0x20dbefe0ul, 0x20ul, 0, 0, 0);
>
> I meant some nice way to decode these magic numbers to be more
> understandable :)

Short term, run it under strace. It should show file names, decode
most of flags and structs.

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: sound: use-after-free in hrtimer_cancel
  2016-06-24 13:48             ` Dmitry Vyukov
@ 2016-06-24 14:35               ` Takashi Iwai
  0 siblings, 0 replies; 13+ messages in thread
From: Takashi Iwai @ 2016-06-24 14:35 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Jaroslav Kysela, alsa-devel, LKML, Sasha Levin,
	Alexander Potapenko, Kostya Serebryany, syzkaller

On Fri, 24 Jun 2016 15:48:53 +0200,
Dmitry Vyukov wrote:
> 
> On Fri, Jun 24, 2016 at 3:33 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > On Tue, 21 Jun 2016 19:41:28 +0200,
> > Dmitry Vyukov wrote:
> >>
> >> On Mon, Jun 6, 2016 at 6:39 PM, Takashi Iwai <tiwai@suse.de> wrote:
> >> > On Mon, 06 Jun 2016 18:29:25 +0200,
> >> > Dmitry Vyukov wrote:
> >> >>
> >> >> On Mon, Jun 6, 2016 at 4:11 PM, Takashi Iwai <tiwai@suse.de> wrote:
> >> >> > On Sat, 04 Jun 2016 20:27:50 +0200,
> >> >> > Dmitry Vyukov wrote:
> >> >> >>
> >> >> >> On Sat, Jun 4, 2016 at 8:00 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> >> >> >> > Hello,
> >> >> >> >
> >> >> >> > The following program triggers use-after-free:
> >> >> >>
> >> >> >> Forget to mention that you need to run it in a tight parallel loop. It
> >> >> >> takes around 5 minutes to reproduce for me.
> >> >> >
> >> >> > Hmm, this again is a bug that is difficult to trigger...  At least, I
> >> >> > couldn't reproduce locally.  How many processes are you running with
> >> >> > stress program?
> >> >>
> >> >> I use a VM with 4 cores and use 20 parallel test processes.
> >> >>
> >> >> > It seems that there is nothing more than opening /dev/audio and does
> >> >> > some mmap in the job.  Is there any other relevant thing there?
> >> >>
> >> >>
> >> >> I think poll with timeout is related. It is poll who sets hrtimer, right?
> >> >
> >> > If it's about snd-dummy driver, hrtimer is created at open, and
> >> > started/stopped at PCM trigger, and removed at close.
> >> >
> >> > Is there any good way to decode which syscalls are executed in the
> >> > test code?
> >>
> >> What do you mean?
> >> Here are the syscalls in the program:
> >>
> >>     r[2] = syscall(SYS_open, "/dev/audio", 0xa40ul, 0, 0, 0);
> >>     // r[2] is in the descriptor passed to SYS_poll
> >>     r[15] = syscall(SYS_poll, 0x2001dde8ul, 0x4ul, 0x8ul, 0, 0, 0);
> >>     r[18] = syscall(SYS_readv, r[2], 0x20dc13c0ul, 0x1ul, 0, 0, 0);
> >>     r[19] = syscall(SYS_read, r[2], 0x20dbefe0ul, 0x20ul, 0, 0, 0);
> >
> > I meant some nice way to decode these magic numbers to be more
> > understandable :)
> 
> Short term, run it under strace. It should show file names, decode
> most of flags and structs.

Alright, thanks.


Takashi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: sound: use-after-free in hrtimer_cancel
  2016-06-24 13:33         ` Dmitry Vyukov
@ 2016-06-24 14:37           ` Takashi Iwai
  0 siblings, 0 replies; 13+ messages in thread
From: Takashi Iwai @ 2016-06-24 14:37 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Jaroslav Kysela, alsa-devel, LKML, Sasha Levin,
	Alexander Potapenko, Kostya Serebryany, syzkaller

On Fri, 24 Jun 2016 15:33:35 +0200,
Dmitry Vyukov wrote:
> 
> On Fri, Jun 24, 2016 at 3:32 PM, Takashi Iwai <tiwai@suse.de> wrote:
> > On Tue, 21 Jun 2016 20:26:48 +0200,
> > Dmitry Vyukov wrote:
> >>
> >> On Mon, Jun 6, 2016 at 4:11 PM, Takashi Iwai <tiwai@suse.de> wrote:
> >> > On Sat, 04 Jun 2016 20:27:50 +0200,
> >> > Dmitry Vyukov wrote:
> >> >>
> >> >> On Sat, Jun 4, 2016 at 8:00 PM, Dmitry Vyukov <dvyukov@google.com> wrote:
> >> >> > Hello,
> >> >> >
> >> >> > The following program triggers use-after-free:
> >> >>
> >> >> Forget to mention that you need to run it in a tight parallel loop. It
> >> >> takes around 5 minutes to reproduce for me.
> >> >
> >> > Hmm, this again is a bug that is difficult to trigger...  At least, I
> >> > couldn't reproduce locally.  How many processes are you running with
> >> > stress program?
> >> >
> >> > It seems that there is nothing more than opening /dev/audio and does
> >> > some mmap in the job.  Is there any other relevant thing there?
> >> >
> >> > Also, this assumes that the first sound card is Dummy driver, right?
> >> > Check /proc/asound/cards.
> >> >
> >> > If it's about snd-dummy driver, one blind shot would be a patch like
> >> > below.  But even if it would fix, it doesn't explain why it's
> >> > triggered in that way...
> >> >
> >> >
> >> > thanks,
> >> >
> >> > Takashi
> >> >
> >> > ---
> >> > diff --git a/sound/drivers/dummy.c b/sound/drivers/dummy.c
> >> > index c0f8f613f1f1..172dacd925f5 100644
> >> > --- a/sound/drivers/dummy.c
> >> > +++ b/sound/drivers/dummy.c
> >> > @@ -420,6 +420,7 @@ static int dummy_hrtimer_stop(struct snd_pcm_substream *substream)
> >> >
> >> >  static inline void dummy_hrtimer_sync(struct dummy_hrtimer_pcm *dpcm)
> >> >  {
> >> > +       hrtimer_cancel(&dpcm->timer);
> >> >         tasklet_kill(&dpcm->tasklet);
> >> >  }
> >>
> >>
> >> Yes, this seems to fix it. I've stressed it for an hour with several
> >> reproducers. If I am not mistaken, it also makes test cases run 15%
> >> faster.
> >
> > Interesting.  Possibly because it now syncs properly before other
> > restart of stream or such...
> >
> >> Please mail a patch.
> >
> > OK, below is the formal patch.
> > Let me know if I can give your tested-by tag.
> 
> Sure.
> 
> Tested-by: Dmitry Vyukov <dvyukov@google.com>

Thanks, the patch queued with your tags, now.


Takashi

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2016-06-24 14:37 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-04 18:00 sound: use-after-free in hrtimer_cancel Dmitry Vyukov
2016-06-04 18:27 ` Dmitry Vyukov
2016-06-06 14:11   ` Takashi Iwai
2016-06-06 16:29     ` Dmitry Vyukov
2016-06-06 16:39       ` Takashi Iwai
2016-06-21 17:41         ` Dmitry Vyukov
2016-06-24 13:33           ` Takashi Iwai
2016-06-24 13:48             ` Dmitry Vyukov
2016-06-24 14:35               ` Takashi Iwai
2016-06-21 18:26     ` Dmitry Vyukov
2016-06-24 13:32       ` Takashi Iwai
2016-06-24 13:33         ` Dmitry Vyukov
2016-06-24 14:37           ` Takashi Iwai

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.