All of lore.kernel.org
 help / color / mirror / Atom feed
* sound: use-after-free in snd_seq_deliver_single_event
@ 2016-02-01 10:26 Dmitry Vyukov
  2016-02-01 10:57   ` Takashi Iwai
  2016-02-01 11:16 ` Mark Brown
  0 siblings, 2 replies; 12+ messages in thread
From: Dmitry Vyukov @ 2016-02-01 10:26 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, Jie Yang, Mark Brown,
	Takashi Sakamoto, alsa-devel, LKML
  Cc: syzkaller, Kostya Serebryany, Alexander Potapenko, Sasha Levin

Hello,

The following program triggers use-after-free in
snd_seq_deliver_single_event (run in a tight parallel loop):

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

#ifndef SYS_mmap
#define SYS_mmap 9
#endif
#ifndef SYS_syz_open_dev
#define SYS_syz_open_dev 1000001
#endif
#ifndef SYS_ioctl
#define SYS_ioctl 16
#endif
#ifndef SYS_getpgrp
#define SYS_getpgrp 111
#endif
#ifndef SYS_migrate_pages
#define SYS_migrate_pages 256
#endif
#ifndef SYS_read
#define SYS_read 0
#endif
#ifndef SYS_write
#define SYS_write 1
#endif
#ifndef SYS_dup3
#define SYS_dup3 292
#endif

long r[59];

void* thr(void* arg)
{
  switch ((long)arg) {
  case 0:
    r[0] = syscall(SYS_mmap, 0x20000000ul, 0x47000ul, 0x3ul, 0x32ul,
                   0xfffffffffffffffful, 0x0ul);
    break;
  case 1:
    memcpy((void*)0x20000000,
           "\x2f\x64\x65\x76\x2f\x73\x65\x71\x75\x65\x6e\x63\x65\x72",
           14);
    r[2] =
        syscall(SYS_open, "/dev/sequencer", 0x2ul, 0, 0, 0);
    break;
  case 2:
    r[3] = syscall(SYS_mmap, 0x20047000ul, 0x1000ul, 0x3ul, 0x32ul,
                   0xfffffffffffffffful, 0x0ul);
    break;
  case 3:
    r[4] = syscall(SYS_ioctl, r[2], 0x540ful, 0x20047ffcul, 0, 0, 0);
    if (r[4] != -1)
      r[5] = *(uint32_t*)0x20047ffc;
    break;
  case 4:
    r[6] = syscall(SYS_getpgrp, r[5], 0, 0, 0, 0, 0);
    break;
  case 5:
    r[7] = syscall(SYS_mmap, 0x20047000ul, 0x1000ul, 0x3ul, 0x32ul,
                   0xfffffffffffffffful, 0x0ul);
    break;
  case 6:
    r[8] = syscall(SYS_mmap, 0x20047000ul, 0x1000ul, 0x3ul, 0x32ul,
                   0xfffffffffffffffful, 0x0ul);
    break;
  case 7:
    *(uint64_t*)0x20047000 = (uint64_t)0x5;
    *(uint64_t*)0x20047000 = (uint64_t)0xfffffffffffffff8;
    r[11] = syscall(SYS_migrate_pages, r[6], 0xfful, 0x20047000ul,
                    0x20047000ul, 0, 0);
    break;
  case 8:
    memcpy((void*)0x20043684,
           "\x2f\x64\x65\x76\x2f\x64\x6d\x6d\x69\x64\x69\x23", 12);
    r[13] = syscall(SYS_open, "/dev/dmmidi2", 0x8a000ul, 0, 0, 0);
    break;
  case 9:
    r[14] = syscall(SYS_read, r[13], 0x20045000ul, 0x36ul, 0, 0, 0);
    break;
  case 10:
    *(uint8_t*)0x2000042c = (uint8_t)0x0;
    *(uint8_t*)0x2000042d = (uint8_t)0x9cf;
    *(uint8_t*)0x2000042e = (uint8_t)0x9;
    *(uint8_t*)0x2000042f = (uint8_t)0x0;
    *(uint64_t*)0x20000444 = (uint64_t)0x0;
    *(uint64_t*)0x2000044c = (uint64_t)0x989680;
    *(uint8_t*)0x20000454 = (uint8_t)0x2;
    *(uint8_t*)0x20000455 = (uint8_t)0x100000000;
    *(uint8_t*)0x20000456 = (uint8_t)0x8d;
    *(uint8_t*)0x20000457 = (uint8_t)0x7;
    *(uint8_t*)0x20000464 = (uint8_t)0x51b2;
    *(uint8_t*)0x20000465 = (uint8_t)0xfffffffffffff15a;
    *(uint8_t*)0x20000466 = (uint8_t)0x4;
    *(uint8_t*)0x20000467 = (uint8_t)0x4;
    *(uint32_t*)0x20000468 = (uint32_t)0xffffffff;
    *(uint8_t*)0x2000046c = (uint8_t)0x5;
    *(uint8_t*)0x2000046d = (uint8_t)0xfffffffffffffff8;
    *(uint8_t*)0x2000046e = (uint8_t)0x401;
    *(uint8_t*)0x2000046f = (uint8_t)0x5;
    *(uint64_t*)0x20000484 = (uint64_t)0x4;
    *(uint64_t*)0x2000048c = (uint64_t)0x0;
    *(uint8_t*)0x20000494 = (uint8_t)0xc2c1;
    *(uint8_t*)0x20000495 = (uint8_t)0x1;
    *(uint8_t*)0x20000496 = (uint8_t)0x6b;
    *(uint8_t*)0x20000497 = (uint8_t)0x6;
    *(uint8_t*)0x200004a0 = (uint8_t)0xfffffffffffffffa;
    *(uint8_t*)0x200004a1 = (uint8_t)0x81;
    *(uint8_t*)0x200004a2 = (uint8_t)0x0;
    *(uint8_t*)0x200004a3 = (uint8_t)0xa6;
    *(uint8_t*)0x200004a4 = (uint8_t)0x400;
    *(uint8_t*)0x200004a5 = (uint8_t)0x3ff;
    *(uint8_t*)0x200004a6 = (uint8_t)0x8;
    *(uint8_t*)0x200004a7 = (uint8_t)0x3;
    *(uint64_t*)0x200004bc = (uint64_t)0x0;
    *(uint64_t*)0x200004c4 = (uint64_t)0x0;
    *(uint8_t*)0x200004cc = (uint8_t)0x9e;
    *(uint8_t*)0x200004cd = (uint8_t)0x7ff;
    *(uint8_t*)0x200004ce = (uint8_t)0x7fff;
    *(uint8_t*)0x200004cf = (uint8_t)0x9;
    *(uint8_t*)0x200004e0 = (uint8_t)0xa;
    *(uint32_t*)0x200004e4 = (uint32_t)0x6;
    *(uint32_t*)0x200004e8 = (uint32_t)0xffffffff;
    r[57] = syscall(SYS_write, r[2], 0x2000042cul, 0x78ul, 0, 0, 0);
    break;
  case 11:
    r[58] = syscall(SYS_dup3, r[13], r[2], 0x80000ul, 0, 0, 0);
    break;
  }
  return 0;
}

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

  memset(r, -1, sizeof(r));
  for (i = 0; i < 12; i++) {
    pthread_create(&th[i], 0, thr, (void*)i);
    usleep(10000);
  }
  for (i = 0; i < 12; i++) {
    pthread_create(&th[i], 0, thr, (void*)i);
    if (i % 2 == 0)
      usleep(10000);
  }
  usleep(100000);
  return 0;
}


==================================================================
BUG: KASAN: use-after-free in do_raw_spin_lock+0x281/0x2b0 at addr
ffff88002abf1654
Read of size 4 by task syz-executor/13987
=============================================================================
BUG kmalloc-96 (Not tainted): kasan: bad access detected
-----------------------------------------------------------------------------

INFO: Allocated in snd_midi_event_new+0x73/0x200 age=10 cpu=0 pid=13996
[<      none      >] ___slab_alloc+0x564/0x5b0 mm/slub.c:2470
[<      none      >] __slab_alloc+0x66/0xc0 mm/slub.c:2499
[<     inline     >] slab_alloc_node mm/slub.c:2562
[<     inline     >] slab_alloc mm/slub.c:2604
[<      none      >] kmem_cache_alloc_trace+0x25c/0x300 mm/slub.c:2621
[<     inline     >] kmalloc include/linux/slab.h:463
[<     inline     >] kzalloc include/linux/slab.h:607
[<      none      >] snd_midi_event_new+0x73/0x200
sound/core/seq/seq_midi_event.c:120
[<      none      >] snd_virmidi_input_open+0xfd/0x380
sound/core/seq/seq_virmidi.c:214
[<      none      >] open_substream+0x3eb/0x780 sound/core/rawmidi.c:269
[<      none      >] rawmidi_open_priv+0x144/0x300 sound/core/rawmidi.c:312
[<      none      >] snd_rawmidi_open+0x3fb/0xa90 sound/core/rawmidi.c:416
[<      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:3254
[<      none      >] path_openat+0xde9/0x5e30 fs/namei.c:3386
[<      none      >] do_filp_open+0x18e/0x250 fs/namei.c:3421
[<      none      >] do_sys_open+0x1fc/0x420 fs/open.c:1022
[<     inline     >] SYSC_open fs/open.c:1040
[<      none      >] SyS_open+0x2d/0x40 fs/open.c:1035

INFO: Freed in snd_midi_event_free+0x43/0x60 age=17 cpu=0 pid=13996
[<      none      >] __slab_free+0x1fc/0x320 mm/slub.c:2680
[<     inline     >] slab_free mm/slub.c:2835
[<      none      >] kfree+0x2ac/0x2c0 mm/slub.c:3664
[<      none      >] snd_midi_event_free+0x43/0x60
sound/core/seq/seq_midi_event.c:142
[<      none      >] snd_virmidi_input_close+0x87/0x120
sound/core/seq/seq_virmidi.c:261
[<      none      >] close_substream.part.14+0xda/0x540 sound/core/rawmidi.c:486
[<     inline     >] close_substream sound/core/rawmidi.c:514
[<      none      >] rawmidi_release_priv+0x1e4/0x250 sound/core/rawmidi.c:504
[<      none      >] snd_rawmidi_release+0x62/0xf0 sound/core/rawmidi.c:539
[<      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+0x8b5/0x2cb0 kernel/exit.c:748
[<      none      >] do_group_exit+0x108/0x330 kernel/exit.c:878
[<      none      >] get_signal+0x5e4/0x14f0 kernel/signal.c:2307
[<      none      >] do_signal+0x83/0x1c90 arch/x86/kernel/signal.c:712
[<      none      >] exit_to_usermode_loop+0x1a5/0x210
arch/x86/entry/common.c:247
[<     inline     >] prepare_exit_to_usermode arch/x86/entry/common.c:282
[<      none      >] syscall_return_slowpath+0x2ba/0x340
arch/x86/entry/common.c:344

INFO: Slab 0xffffea0000aafc00 objects=28 used=11 fp=0xffff88002abf0470
flags=0x1fffc0000004080
INFO: Object 0xffff88002abf1630 @offset=5680 fp=0xffff88002abf06a8
CPU: 1 PID: 13987 Comm: syz-executor Tainted: G    B           4.5.0-rc1+ #305
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
 00000000ffffffff ffff880061aaf7d8 ffffffff82be11ad ffff88003e804900
 ffff88002abf1630 ffff88002abf0000 ffff880061aaf808 ffffffff8175b454
 ffff88003e804900 ffffea0000aafc00 ffff88002abf1630 000000000000000a

Call Trace:
 [<ffffffff81764e3e>] __asan_report_load4_noabort+0x3e/0x40
mm/kasan/report.c:294
 [<     inline     >] debug_spin_lock_before kernel/locking/spinlock_debug.c:83
 [<ffffffff814663d1>] do_raw_spin_lock+0x281/0x2b0
kernel/locking/spinlock_debug.c:135
 [<     inline     >] __raw_spin_lock_irqsave
include/linux/spinlock_api_smp.h:119
 [<ffffffff86652b67>] _raw_spin_lock_irqsave+0xa7/0xd0
kernel/locking/spinlock.c:159
 [<ffffffff852bb6a7>] snd_midi_event_decode+0x1f7/0x5c0
sound/core/seq/seq_midi_event.c:391
 [<     inline     >] snd_virmidi_dev_receive_event
sound/core/seq/seq_virmidi.c:95
 [<ffffffff852ce754>] snd_virmidi_event_input+0x214/0x310
sound/core/seq/seq_virmidi.c:133
 [<ffffffff852a7934>]
snd_seq_deliver_single_event.constprop.11+0x3f4/0x740
sound/core/seq/seq_clientmgr.c:634
 [<ffffffff852a7da2>] snd_seq_deliver_event+0x122/0x800
sound/core/seq/seq_clientmgr.c:828
 [<ffffffff852a9186>] snd_seq_kernel_client_dispatch+0x126/0x170
sound/core/seq/seq_clientmgr.c:2401
 [<     inline     >] snd_seq_oss_dispatch
sound/core/seq/oss/seq_oss_device.h:151
 [<ffffffff852ca423>] snd_seq_oss_midi_reset+0x303/0x4a0
sound/core/seq/oss/seq_oss_midi.c:481
 [<ffffffff852bd490>] snd_seq_oss_reset+0x130/0x260
sound/core/seq/oss/seq_oss_init.c:469
 [<ffffffff852bd631>] snd_seq_oss_release+0x71/0x130
sound/core/seq/oss/seq_oss_init.c:425
 [<ffffffff852bbcca>] odev_release+0x5a/0x80 sound/core/seq/oss/seq_oss.c:155
 [<ffffffff817c0156>] __fput+0x236/0x780 fs/file_table.c:208
 [<ffffffff817c0725>] ____fput+0x15/0x20 fs/file_table.c:244
 [<ffffffff813b14e0>] task_work_run+0x170/0x210 kernel/task_work.c:115
 [<     inline     >] tracehook_notify_resume include/linux/tracehook.h:191
 [<ffffffff810066b1>] exit_to_usermode_loop+0x1d1/0x210
arch/x86/entry/common.c:251
 [<     inline     >] prepare_exit_to_usermode arch/x86/entry/common.c:282
 [<ffffffff810084ea>] syscall_return_slowpath+0x2ba/0x340
arch/x86/entry/common.c:344
 [<ffffffff86653322>] int_ret_from_sys_call+0x25/0x9f
arch/x86/entry/entry_64.S:281
==================================================================


general protection fault: 0000 [#1] SMP DEBUG_PAGEALLOC KASAN
Modules linked in:
CPU: 1 PID: 14982 Comm: syz-executor Tainted: G        W       4.5.0-rc1+ #305
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
task: ffff88005e362f80 ti: ffff88005fba8000 task.ti: ffff88005fba8000
RIP: 0010:[<ffffffff852ce6d5>]  [<ffffffff852ce6d5>]
snd_virmidi_event_input+0x195/0x310
RSP: 0018:ffff88005fbaf608  EFLAGS: 00010202
RAX: 1bd5a00000000023 RBX: dead000000000100 RCX: 0000000000000002
RDX: 0000000000000004 RSI: 0000000000000100 RDI: dead00000000011c
RBP: ffff88005fbaf6b8 R08: 0000000000000001 R09: 0000000000000000
R10: ffff88005e362f80 R11: ffff8800009fe110 R12: dffffc0000000000
R13: ffff88005fbaf900 R14: ffff8800009fe130 R15: 1ffff1000bf75ec6
FS:  00007fb93c201700(0000) GS:ffff88003ed00000(0000) knlGS:0000000000000000
CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
CR2: 0000000020045000 CR3: 000000005f9bd000 CR4: 00000000000006e0
Stack:
 ffff8800009fe0f8 ffffed000bf75f20 ffff88005fbaf901 0000000000000000
 ffff88003639fa50 0000000041b58ab3 ffffffff879486ce ffffffff852ce540
 ffff88003639fa28 0000000000000000 ffff88003639fa38 ffff88005fbaf678
Call Trace:
 [<ffffffff852a7934>]
snd_seq_deliver_single_event.constprop.11+0x3f4/0x740
sound/core/seq/seq_clientmgr.c:634
 [<ffffffff852a7da2>] snd_seq_deliver_event+0x122/0x800
sound/core/seq/seq_clientmgr.c:828
 [<ffffffff852a9186>] snd_seq_kernel_client_dispatch+0x126/0x170
sound/core/seq/seq_clientmgr.c:2401
 [<     inline     >] snd_seq_oss_dispatch
sound/core/seq/oss/seq_oss_device.h:151
 [<ffffffff852ca527>] snd_seq_oss_midi_reset+0x407/0x4a0
sound/core/seq/oss/seq_oss_midi.c:475
 [<ffffffff852bd490>] snd_seq_oss_reset+0x130/0x260
sound/core/seq/oss/seq_oss_init.c:469
 [<ffffffff852bd631>] snd_seq_oss_release+0x71/0x130
sound/core/seq/oss/seq_oss_init.c:425
 [<ffffffff852bbcca>] odev_release+0x5a/0x80 sound/core/seq/oss/seq_oss.c:155
 [<ffffffff817c0156>] __fput+0x236/0x780 fs/file_table.c:208
 [<ffffffff817c0725>] ____fput+0x15/0x20 fs/file_table.c:244
 [<ffffffff813b14e0>] task_work_run+0x170/0x210 kernel/task_work.c:115
 [<     inline     >] exit_task_work include/linux/task_work.h:21
 [<ffffffff8135ca55>] do_exit+0x8b5/0x2cb0 kernel/exit.c:748
 [<ffffffff8135efc8>] do_group_exit+0x108/0x330 kernel/exit.c:878
 [<ffffffff81382144>] get_signal+0x5e4/0x14f0 kernel/signal.c:2307
 [<ffffffff811a0db3>] do_signal+0x83/0x1c90 arch/x86/kernel/signal.c:712
 [<ffffffff81006685>] exit_to_usermode_loop+0x1a5/0x210
arch/x86/entry/common.c:247
 [<     inline     >] prepare_exit_to_usermode arch/x86/entry/common.c:282
 [<ffffffff810084ea>] syscall_return_slowpath+0x2ba/0x340
arch/x86/entry/common.c:344
 [<ffffffff86653322>] int_ret_from_sys_call+0x25/0x9f
arch/x86/entry/entry_64.S:281
Code: 3c 20 00 0f 85 31 01 00 00 48 8b 1b 49 39 de 0f 84 d9 00 00 00
e8 ec bb 29 fc 48 8d 7b 1c 48 89 f8 48 89 fa 48 c1 e8 03 83 e2 07 <42>
0f b6 04 20 38 d0 7f 08 84 c0 0f 85 3d 01 00 00 f6 43 1c 01
RIP  [<     inline     >] snd_virmidi_dev_receive_event
sound/core/seq/seq_virmidi.c:88
RIP  [<ffffffff852ce6d5>] snd_virmidi_event_input+0x195/0x310
sound/core/seq/seq_virmidi.c:133
 RSP <ffff88005fbaf608>
---[ end trace 35fa82e449e91fba ]---
Fixing recursive fault but reboot is needed!

On commit 26cd83670f2f5a3d5b5514a1f7d96567cdb9558b.

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

* Re: sound: use-after-free in snd_seq_deliver_single_event
  2016-02-01 10:26 sound: use-after-free in snd_seq_deliver_single_event Dmitry Vyukov
@ 2016-02-01 10:57   ` Takashi Iwai
  2016-02-01 11:16 ` Mark Brown
  1 sibling, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2016-02-01 10:57 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: alsa-devel, Jie Yang, Mark Brown, Jaroslav Kysela,
	Takashi Sakamoto, LKML, Alexander Potapenko, Kostya Serebryany,
	syzkaller, Sasha Levin

On Mon, 01 Feb 2016 11:26:57 +0100,
Dmitry Vyukov wrote:
> 
> Hello,
> 
> The following program triggers use-after-free in
> snd_seq_deliver_single_event (run in a tight parallel loop):
> 
> // autogenerated by syzkaller (http://github.com/google/syzkaller)
> #include <pthread.h>
> #include <stdint.h>
> #include <string.h>
> #include <sys/syscall.h>
> #include <unistd.h>
> 
> #ifndef SYS_mmap
> #define SYS_mmap 9
> #endif
> #ifndef SYS_syz_open_dev
> #define SYS_syz_open_dev 1000001
> #endif
> #ifndef SYS_ioctl
> #define SYS_ioctl 16
> #endif
> #ifndef SYS_getpgrp
> #define SYS_getpgrp 111
> #endif
> #ifndef SYS_migrate_pages
> #define SYS_migrate_pages 256
> #endif
> #ifndef SYS_read
> #define SYS_read 0
> #endif
> #ifndef SYS_write
> #define SYS_write 1
> #endif
> #ifndef SYS_dup3
> #define SYS_dup3 292
> #endif
> 
> long r[59];
> 
> void* thr(void* arg)
> {
>   switch ((long)arg) {
>   case 0:
>     r[0] = syscall(SYS_mmap, 0x20000000ul, 0x47000ul, 0x3ul, 0x32ul,
>                    0xfffffffffffffffful, 0x0ul);
>     break;
>   case 1:
>     memcpy((void*)0x20000000,
>            "\x2f\x64\x65\x76\x2f\x73\x65\x71\x75\x65\x6e\x63\x65\x72",
>            14);
>     r[2] =
>         syscall(SYS_open, "/dev/sequencer", 0x2ul, 0, 0, 0);
>     break;
>   case 2:
>     r[3] = syscall(SYS_mmap, 0x20047000ul, 0x1000ul, 0x3ul, 0x32ul,
>                    0xfffffffffffffffful, 0x0ul);
>     break;
>   case 3:
>     r[4] = syscall(SYS_ioctl, r[2], 0x540ful, 0x20047ffcul, 0, 0, 0);
>     if (r[4] != -1)
>       r[5] = *(uint32_t*)0x20047ffc;
>     break;
>   case 4:
>     r[6] = syscall(SYS_getpgrp, r[5], 0, 0, 0, 0, 0);
>     break;
>   case 5:
>     r[7] = syscall(SYS_mmap, 0x20047000ul, 0x1000ul, 0x3ul, 0x32ul,
>                    0xfffffffffffffffful, 0x0ul);
>     break;
>   case 6:
>     r[8] = syscall(SYS_mmap, 0x20047000ul, 0x1000ul, 0x3ul, 0x32ul,
>                    0xfffffffffffffffful, 0x0ul);
>     break;
>   case 7:
>     *(uint64_t*)0x20047000 = (uint64_t)0x5;
>     *(uint64_t*)0x20047000 = (uint64_t)0xfffffffffffffff8;
>     r[11] = syscall(SYS_migrate_pages, r[6], 0xfful, 0x20047000ul,
>                     0x20047000ul, 0, 0);
>     break;
>   case 8:
>     memcpy((void*)0x20043684,
>            "\x2f\x64\x65\x76\x2f\x64\x6d\x6d\x69\x64\x69\x23", 12);
>     r[13] = syscall(SYS_open, "/dev/dmmidi2", 0x8a000ul, 0, 0, 0);
>     break;
>   case 9:
>     r[14] = syscall(SYS_read, r[13], 0x20045000ul, 0x36ul, 0, 0, 0);
>     break;
>   case 10:
>     *(uint8_t*)0x2000042c = (uint8_t)0x0;
>     *(uint8_t*)0x2000042d = (uint8_t)0x9cf;
>     *(uint8_t*)0x2000042e = (uint8_t)0x9;
>     *(uint8_t*)0x2000042f = (uint8_t)0x0;
>     *(uint64_t*)0x20000444 = (uint64_t)0x0;
>     *(uint64_t*)0x2000044c = (uint64_t)0x989680;
>     *(uint8_t*)0x20000454 = (uint8_t)0x2;
>     *(uint8_t*)0x20000455 = (uint8_t)0x100000000;
>     *(uint8_t*)0x20000456 = (uint8_t)0x8d;
>     *(uint8_t*)0x20000457 = (uint8_t)0x7;
>     *(uint8_t*)0x20000464 = (uint8_t)0x51b2;
>     *(uint8_t*)0x20000465 = (uint8_t)0xfffffffffffff15a;
>     *(uint8_t*)0x20000466 = (uint8_t)0x4;
>     *(uint8_t*)0x20000467 = (uint8_t)0x4;
>     *(uint32_t*)0x20000468 = (uint32_t)0xffffffff;
>     *(uint8_t*)0x2000046c = (uint8_t)0x5;
>     *(uint8_t*)0x2000046d = (uint8_t)0xfffffffffffffff8;
>     *(uint8_t*)0x2000046e = (uint8_t)0x401;
>     *(uint8_t*)0x2000046f = (uint8_t)0x5;
>     *(uint64_t*)0x20000484 = (uint64_t)0x4;
>     *(uint64_t*)0x2000048c = (uint64_t)0x0;
>     *(uint8_t*)0x20000494 = (uint8_t)0xc2c1;
>     *(uint8_t*)0x20000495 = (uint8_t)0x1;
>     *(uint8_t*)0x20000496 = (uint8_t)0x6b;
>     *(uint8_t*)0x20000497 = (uint8_t)0x6;
>     *(uint8_t*)0x200004a0 = (uint8_t)0xfffffffffffffffa;
>     *(uint8_t*)0x200004a1 = (uint8_t)0x81;
>     *(uint8_t*)0x200004a2 = (uint8_t)0x0;
>     *(uint8_t*)0x200004a3 = (uint8_t)0xa6;
>     *(uint8_t*)0x200004a4 = (uint8_t)0x400;
>     *(uint8_t*)0x200004a5 = (uint8_t)0x3ff;
>     *(uint8_t*)0x200004a6 = (uint8_t)0x8;
>     *(uint8_t*)0x200004a7 = (uint8_t)0x3;
>     *(uint64_t*)0x200004bc = (uint64_t)0x0;
>     *(uint64_t*)0x200004c4 = (uint64_t)0x0;
>     *(uint8_t*)0x200004cc = (uint8_t)0x9e;
>     *(uint8_t*)0x200004cd = (uint8_t)0x7ff;
>     *(uint8_t*)0x200004ce = (uint8_t)0x7fff;
>     *(uint8_t*)0x200004cf = (uint8_t)0x9;
>     *(uint8_t*)0x200004e0 = (uint8_t)0xa;
>     *(uint32_t*)0x200004e4 = (uint32_t)0x6;
>     *(uint32_t*)0x200004e8 = (uint32_t)0xffffffff;
>     r[57] = syscall(SYS_write, r[2], 0x2000042cul, 0x78ul, 0, 0, 0);
>     break;
>   case 11:
>     r[58] = syscall(SYS_dup3, r[13], r[2], 0x80000ul, 0, 0, 0);
>     break;
>   }
>   return 0;
> }
> 
> int main()
> {
>   long i;
>   pthread_t th[12];
> 
>   memset(r, -1, sizeof(r));
>   for (i = 0; i < 12; i++) {
>     pthread_create(&th[i], 0, thr, (void*)i);
>     usleep(10000);
>   }
>   for (i = 0; i < 12; i++) {
>     pthread_create(&th[i], 0, thr, (void*)i);
>     if (i % 2 == 0)
>       usleep(10000);
>   }
>   usleep(100000);
>   return 0;
> }
> 
> 
> ==================================================================
> BUG: KASAN: use-after-free in do_raw_spin_lock+0x281/0x2b0 at addr
> ffff88002abf1654
> Read of size 4 by task syz-executor/13987
> =============================================================================
> BUG kmalloc-96 (Not tainted): kasan: bad access detected
> -----------------------------------------------------------------------------
> 
> INFO: Allocated in snd_midi_event_new+0x73/0x200 age=10 cpu=0 pid=13996
> [<      none      >] ___slab_alloc+0x564/0x5b0 mm/slub.c:2470
> [<      none      >] __slab_alloc+0x66/0xc0 mm/slub.c:2499
> [<     inline     >] slab_alloc_node mm/slub.c:2562
> [<     inline     >] slab_alloc mm/slub.c:2604
> [<      none      >] kmem_cache_alloc_trace+0x25c/0x300 mm/slub.c:2621
> [<     inline     >] kmalloc include/linux/slab.h:463
> [<     inline     >] kzalloc include/linux/slab.h:607
> [<      none      >] snd_midi_event_new+0x73/0x200
> sound/core/seq/seq_midi_event.c:120
> [<      none      >] snd_virmidi_input_open+0xfd/0x380
> sound/core/seq/seq_virmidi.c:214
> [<      none      >] open_substream+0x3eb/0x780 sound/core/rawmidi.c:269
> [<      none      >] rawmidi_open_priv+0x144/0x300 sound/core/rawmidi.c:312
> [<      none      >] snd_rawmidi_open+0x3fb/0xa90 sound/core/rawmidi.c:416
> [<      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:3254
> [<      none      >] path_openat+0xde9/0x5e30 fs/namei.c:3386
> [<      none      >] do_filp_open+0x18e/0x250 fs/namei.c:3421
> [<      none      >] do_sys_open+0x1fc/0x420 fs/open.c:1022
> [<     inline     >] SYSC_open fs/open.c:1040
> [<      none      >] SyS_open+0x2d/0x40 fs/open.c:1035
> 
> INFO: Freed in snd_midi_event_free+0x43/0x60 age=17 cpu=0 pid=13996
> [<      none      >] __slab_free+0x1fc/0x320 mm/slub.c:2680
> [<     inline     >] slab_free mm/slub.c:2835
> [<      none      >] kfree+0x2ac/0x2c0 mm/slub.c:3664
> [<      none      >] snd_midi_event_free+0x43/0x60
> sound/core/seq/seq_midi_event.c:142
> [<      none      >] snd_virmidi_input_close+0x87/0x120
> sound/core/seq/seq_virmidi.c:261
> [<      none      >] close_substream.part.14+0xda/0x540 sound/core/rawmidi.c:486
> [<     inline     >] close_substream sound/core/rawmidi.c:514
> [<      none      >] rawmidi_release_priv+0x1e4/0x250 sound/core/rawmidi.c:504
> [<      none      >] snd_rawmidi_release+0x62/0xf0 sound/core/rawmidi.c:539
> [<      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+0x8b5/0x2cb0 kernel/exit.c:748
> [<      none      >] do_group_exit+0x108/0x330 kernel/exit.c:878
> [<      none      >] get_signal+0x5e4/0x14f0 kernel/signal.c:2307
> [<      none      >] do_signal+0x83/0x1c90 arch/x86/kernel/signal.c:712
> [<      none      >] exit_to_usermode_loop+0x1a5/0x210
> arch/x86/entry/common.c:247
> [<     inline     >] prepare_exit_to_usermode arch/x86/entry/common.c:282
> [<      none      >] syscall_return_slowpath+0x2ba/0x340
> arch/x86/entry/common.c:344
> 
> INFO: Slab 0xffffea0000aafc00 objects=28 used=11 fp=0xffff88002abf0470
> flags=0x1fffc0000004080
> INFO: Object 0xffff88002abf1630 @offset=5680 fp=0xffff88002abf06a8
> CPU: 1 PID: 13987 Comm: syz-executor Tainted: G    B           4.5.0-rc1+ #305
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>  00000000ffffffff ffff880061aaf7d8 ffffffff82be11ad ffff88003e804900
>  ffff88002abf1630 ffff88002abf0000 ffff880061aaf808 ffffffff8175b454
>  ffff88003e804900 ffffea0000aafc00 ffff88002abf1630 000000000000000a
> 
> Call Trace:
>  [<ffffffff81764e3e>] __asan_report_load4_noabort+0x3e/0x40
> mm/kasan/report.c:294
>  [<     inline     >] debug_spin_lock_before kernel/locking/spinlock_debug.c:83
>  [<ffffffff814663d1>] do_raw_spin_lock+0x281/0x2b0
> kernel/locking/spinlock_debug.c:135
>  [<     inline     >] __raw_spin_lock_irqsave
> include/linux/spinlock_api_smp.h:119
>  [<ffffffff86652b67>] _raw_spin_lock_irqsave+0xa7/0xd0
> kernel/locking/spinlock.c:159
>  [<ffffffff852bb6a7>] snd_midi_event_decode+0x1f7/0x5c0
> sound/core/seq/seq_midi_event.c:391
>  [<     inline     >] snd_virmidi_dev_receive_event
> sound/core/seq/seq_virmidi.c:95
>  [<ffffffff852ce754>] snd_virmidi_event_input+0x214/0x310
> sound/core/seq/seq_virmidi.c:133
>  [<ffffffff852a7934>]
> snd_seq_deliver_single_event.constprop.11+0x3f4/0x740
> sound/core/seq/seq_clientmgr.c:634
>  [<ffffffff852a7da2>] snd_seq_deliver_event+0x122/0x800
> sound/core/seq/seq_clientmgr.c:828
>  [<ffffffff852a9186>] snd_seq_kernel_client_dispatch+0x126/0x170
> sound/core/seq/seq_clientmgr.c:2401
>  [<     inline     >] snd_seq_oss_dispatch
> sound/core/seq/oss/seq_oss_device.h:151
>  [<ffffffff852ca423>] snd_seq_oss_midi_reset+0x303/0x4a0
> sound/core/seq/oss/seq_oss_midi.c:481
>  [<ffffffff852bd490>] snd_seq_oss_reset+0x130/0x260
> sound/core/seq/oss/seq_oss_init.c:469
>  [<ffffffff852bd631>] snd_seq_oss_release+0x71/0x130
> sound/core/seq/oss/seq_oss_init.c:425
>  [<ffffffff852bbcca>] odev_release+0x5a/0x80 sound/core/seq/oss/seq_oss.c:155
>  [<ffffffff817c0156>] __fput+0x236/0x780 fs/file_table.c:208
>  [<ffffffff817c0725>] ____fput+0x15/0x20 fs/file_table.c:244
>  [<ffffffff813b14e0>] task_work_run+0x170/0x210 kernel/task_work.c:115
>  [<     inline     >] tracehook_notify_resume include/linux/tracehook.h:191
>  [<ffffffff810066b1>] exit_to_usermode_loop+0x1d1/0x210
> arch/x86/entry/common.c:251
>  [<     inline     >] prepare_exit_to_usermode arch/x86/entry/common.c:282
>  [<ffffffff810084ea>] syscall_return_slowpath+0x2ba/0x340
> arch/x86/entry/common.c:344
>  [<ffffffff86653322>] int_ret_from_sys_call+0x25/0x9f
> arch/x86/entry/entry_64.S:281

Looks like a race at closing virmidi device.
Does the patch below fix it?


thanks,

Takashi

---
diff --git a/sound/core/seq/seq_virmidi.c b/sound/core/seq/seq_virmidi.c
index 3da2d48610b3..f71aedfb408c 100644
--- a/sound/core/seq/seq_virmidi.c
+++ b/sound/core/seq/seq_virmidi.c
@@ -254,9 +254,13 @@ static int snd_virmidi_output_open(struct snd_rawmidi_substream *substream)
  */
 static int snd_virmidi_input_close(struct snd_rawmidi_substream *substream)
 {
+	struct snd_virmidi_dev *rdev = substream->rmidi->private_data;
 	struct snd_virmidi *vmidi = substream->runtime->private_data;
-	snd_midi_event_free(vmidi->parser);
+
+	write_lock_irq(&rdev->filelist_lock);
 	list_del(&vmidi->list);
+	write_unlock_irq(&rdev->filelist_lock);
+	snd_midi_event_free(vmidi->parser);
 	substream->runtime->private_data = NULL;
 	kfree(vmidi);
 	return 0;

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

* Re: sound: use-after-free in snd_seq_deliver_single_event
@ 2016-02-01 10:57   ` Takashi Iwai
  0 siblings, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2016-02-01 10:57 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: alsa-devel, Jie Yang, Mark Brown, Jaroslav Kysela,
	Takashi Sakamoto, LKML, Alexander Potapenko, Kostya Serebryany,
	syzkaller, Sasha Levin

On Mon, 01 Feb 2016 11:26:57 +0100,
Dmitry Vyukov wrote:
> 
> Hello,
> 
> The following program triggers use-after-free in
> snd_seq_deliver_single_event (run in a tight parallel loop):
> 
> // autogenerated by syzkaller (http://github.com/google/syzkaller)
> #include <pthread.h>
> #include <stdint.h>
> #include <string.h>
> #include <sys/syscall.h>
> #include <unistd.h>
> 
> #ifndef SYS_mmap
> #define SYS_mmap 9
> #endif
> #ifndef SYS_syz_open_dev
> #define SYS_syz_open_dev 1000001
> #endif
> #ifndef SYS_ioctl
> #define SYS_ioctl 16
> #endif
> #ifndef SYS_getpgrp
> #define SYS_getpgrp 111
> #endif
> #ifndef SYS_migrate_pages
> #define SYS_migrate_pages 256
> #endif
> #ifndef SYS_read
> #define SYS_read 0
> #endif
> #ifndef SYS_write
> #define SYS_write 1
> #endif
> #ifndef SYS_dup3
> #define SYS_dup3 292
> #endif
> 
> long r[59];
> 
> void* thr(void* arg)
> {
>   switch ((long)arg) {
>   case 0:
>     r[0] = syscall(SYS_mmap, 0x20000000ul, 0x47000ul, 0x3ul, 0x32ul,
>                    0xfffffffffffffffful, 0x0ul);
>     break;
>   case 1:
>     memcpy((void*)0x20000000,
>            "\x2f\x64\x65\x76\x2f\x73\x65\x71\x75\x65\x6e\x63\x65\x72",
>            14);
>     r[2] =
>         syscall(SYS_open, "/dev/sequencer", 0x2ul, 0, 0, 0);
>     break;
>   case 2:
>     r[3] = syscall(SYS_mmap, 0x20047000ul, 0x1000ul, 0x3ul, 0x32ul,
>                    0xfffffffffffffffful, 0x0ul);
>     break;
>   case 3:
>     r[4] = syscall(SYS_ioctl, r[2], 0x540ful, 0x20047ffcul, 0, 0, 0);
>     if (r[4] != -1)
>       r[5] = *(uint32_t*)0x20047ffc;
>     break;
>   case 4:
>     r[6] = syscall(SYS_getpgrp, r[5], 0, 0, 0, 0, 0);
>     break;
>   case 5:
>     r[7] = syscall(SYS_mmap, 0x20047000ul, 0x1000ul, 0x3ul, 0x32ul,
>                    0xfffffffffffffffful, 0x0ul);
>     break;
>   case 6:
>     r[8] = syscall(SYS_mmap, 0x20047000ul, 0x1000ul, 0x3ul, 0x32ul,
>                    0xfffffffffffffffful, 0x0ul);
>     break;
>   case 7:
>     *(uint64_t*)0x20047000 = (uint64_t)0x5;
>     *(uint64_t*)0x20047000 = (uint64_t)0xfffffffffffffff8;
>     r[11] = syscall(SYS_migrate_pages, r[6], 0xfful, 0x20047000ul,
>                     0x20047000ul, 0, 0);
>     break;
>   case 8:
>     memcpy((void*)0x20043684,
>            "\x2f\x64\x65\x76\x2f\x64\x6d\x6d\x69\x64\x69\x23", 12);
>     r[13] = syscall(SYS_open, "/dev/dmmidi2", 0x8a000ul, 0, 0, 0);
>     break;
>   case 9:
>     r[14] = syscall(SYS_read, r[13], 0x20045000ul, 0x36ul, 0, 0, 0);
>     break;
>   case 10:
>     *(uint8_t*)0x2000042c = (uint8_t)0x0;
>     *(uint8_t*)0x2000042d = (uint8_t)0x9cf;
>     *(uint8_t*)0x2000042e = (uint8_t)0x9;
>     *(uint8_t*)0x2000042f = (uint8_t)0x0;
>     *(uint64_t*)0x20000444 = (uint64_t)0x0;
>     *(uint64_t*)0x2000044c = (uint64_t)0x989680;
>     *(uint8_t*)0x20000454 = (uint8_t)0x2;
>     *(uint8_t*)0x20000455 = (uint8_t)0x100000000;
>     *(uint8_t*)0x20000456 = (uint8_t)0x8d;
>     *(uint8_t*)0x20000457 = (uint8_t)0x7;
>     *(uint8_t*)0x20000464 = (uint8_t)0x51b2;
>     *(uint8_t*)0x20000465 = (uint8_t)0xfffffffffffff15a;
>     *(uint8_t*)0x20000466 = (uint8_t)0x4;
>     *(uint8_t*)0x20000467 = (uint8_t)0x4;
>     *(uint32_t*)0x20000468 = (uint32_t)0xffffffff;
>     *(uint8_t*)0x2000046c = (uint8_t)0x5;
>     *(uint8_t*)0x2000046d = (uint8_t)0xfffffffffffffff8;
>     *(uint8_t*)0x2000046e = (uint8_t)0x401;
>     *(uint8_t*)0x2000046f = (uint8_t)0x5;
>     *(uint64_t*)0x20000484 = (uint64_t)0x4;
>     *(uint64_t*)0x2000048c = (uint64_t)0x0;
>     *(uint8_t*)0x20000494 = (uint8_t)0xc2c1;
>     *(uint8_t*)0x20000495 = (uint8_t)0x1;
>     *(uint8_t*)0x20000496 = (uint8_t)0x6b;
>     *(uint8_t*)0x20000497 = (uint8_t)0x6;
>     *(uint8_t*)0x200004a0 = (uint8_t)0xfffffffffffffffa;
>     *(uint8_t*)0x200004a1 = (uint8_t)0x81;
>     *(uint8_t*)0x200004a2 = (uint8_t)0x0;
>     *(uint8_t*)0x200004a3 = (uint8_t)0xa6;
>     *(uint8_t*)0x200004a4 = (uint8_t)0x400;
>     *(uint8_t*)0x200004a5 = (uint8_t)0x3ff;
>     *(uint8_t*)0x200004a6 = (uint8_t)0x8;
>     *(uint8_t*)0x200004a7 = (uint8_t)0x3;
>     *(uint64_t*)0x200004bc = (uint64_t)0x0;
>     *(uint64_t*)0x200004c4 = (uint64_t)0x0;
>     *(uint8_t*)0x200004cc = (uint8_t)0x9e;
>     *(uint8_t*)0x200004cd = (uint8_t)0x7ff;
>     *(uint8_t*)0x200004ce = (uint8_t)0x7fff;
>     *(uint8_t*)0x200004cf = (uint8_t)0x9;
>     *(uint8_t*)0x200004e0 = (uint8_t)0xa;
>     *(uint32_t*)0x200004e4 = (uint32_t)0x6;
>     *(uint32_t*)0x200004e8 = (uint32_t)0xffffffff;
>     r[57] = syscall(SYS_write, r[2], 0x2000042cul, 0x78ul, 0, 0, 0);
>     break;
>   case 11:
>     r[58] = syscall(SYS_dup3, r[13], r[2], 0x80000ul, 0, 0, 0);
>     break;
>   }
>   return 0;
> }
> 
> int main()
> {
>   long i;
>   pthread_t th[12];
> 
>   memset(r, -1, sizeof(r));
>   for (i = 0; i < 12; i++) {
>     pthread_create(&th[i], 0, thr, (void*)i);
>     usleep(10000);
>   }
>   for (i = 0; i < 12; i++) {
>     pthread_create(&th[i], 0, thr, (void*)i);
>     if (i % 2 == 0)
>       usleep(10000);
>   }
>   usleep(100000);
>   return 0;
> }
> 
> 
> ==================================================================
> BUG: KASAN: use-after-free in do_raw_spin_lock+0x281/0x2b0 at addr
> ffff88002abf1654
> Read of size 4 by task syz-executor/13987
> =============================================================================
> BUG kmalloc-96 (Not tainted): kasan: bad access detected
> -----------------------------------------------------------------------------
> 
> INFO: Allocated in snd_midi_event_new+0x73/0x200 age=10 cpu=0 pid=13996
> [<      none      >] ___slab_alloc+0x564/0x5b0 mm/slub.c:2470
> [<      none      >] __slab_alloc+0x66/0xc0 mm/slub.c:2499
> [<     inline     >] slab_alloc_node mm/slub.c:2562
> [<     inline     >] slab_alloc mm/slub.c:2604
> [<      none      >] kmem_cache_alloc_trace+0x25c/0x300 mm/slub.c:2621
> [<     inline     >] kmalloc include/linux/slab.h:463
> [<     inline     >] kzalloc include/linux/slab.h:607
> [<      none      >] snd_midi_event_new+0x73/0x200
> sound/core/seq/seq_midi_event.c:120
> [<      none      >] snd_virmidi_input_open+0xfd/0x380
> sound/core/seq/seq_virmidi.c:214
> [<      none      >] open_substream+0x3eb/0x780 sound/core/rawmidi.c:269
> [<      none      >] rawmidi_open_priv+0x144/0x300 sound/core/rawmidi.c:312
> [<      none      >] snd_rawmidi_open+0x3fb/0xa90 sound/core/rawmidi.c:416
> [<      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:3254
> [<      none      >] path_openat+0xde9/0x5e30 fs/namei.c:3386
> [<      none      >] do_filp_open+0x18e/0x250 fs/namei.c:3421
> [<      none      >] do_sys_open+0x1fc/0x420 fs/open.c:1022
> [<     inline     >] SYSC_open fs/open.c:1040
> [<      none      >] SyS_open+0x2d/0x40 fs/open.c:1035
> 
> INFO: Freed in snd_midi_event_free+0x43/0x60 age=17 cpu=0 pid=13996
> [<      none      >] __slab_free+0x1fc/0x320 mm/slub.c:2680
> [<     inline     >] slab_free mm/slub.c:2835
> [<      none      >] kfree+0x2ac/0x2c0 mm/slub.c:3664
> [<      none      >] snd_midi_event_free+0x43/0x60
> sound/core/seq/seq_midi_event.c:142
> [<      none      >] snd_virmidi_input_close+0x87/0x120
> sound/core/seq/seq_virmidi.c:261
> [<      none      >] close_substream.part.14+0xda/0x540 sound/core/rawmidi.c:486
> [<     inline     >] close_substream sound/core/rawmidi.c:514
> [<      none      >] rawmidi_release_priv+0x1e4/0x250 sound/core/rawmidi.c:504
> [<      none      >] snd_rawmidi_release+0x62/0xf0 sound/core/rawmidi.c:539
> [<      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+0x8b5/0x2cb0 kernel/exit.c:748
> [<      none      >] do_group_exit+0x108/0x330 kernel/exit.c:878
> [<      none      >] get_signal+0x5e4/0x14f0 kernel/signal.c:2307
> [<      none      >] do_signal+0x83/0x1c90 arch/x86/kernel/signal.c:712
> [<      none      >] exit_to_usermode_loop+0x1a5/0x210
> arch/x86/entry/common.c:247
> [<     inline     >] prepare_exit_to_usermode arch/x86/entry/common.c:282
> [<      none      >] syscall_return_slowpath+0x2ba/0x340
> arch/x86/entry/common.c:344
> 
> INFO: Slab 0xffffea0000aafc00 objects=28 used=11 fp=0xffff88002abf0470
> flags=0x1fffc0000004080
> INFO: Object 0xffff88002abf1630 @offset=5680 fp=0xffff88002abf06a8
> CPU: 1 PID: 13987 Comm: syz-executor Tainted: G    B           4.5.0-rc1+ #305
> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>  00000000ffffffff ffff880061aaf7d8 ffffffff82be11ad ffff88003e804900
>  ffff88002abf1630 ffff88002abf0000 ffff880061aaf808 ffffffff8175b454
>  ffff88003e804900 ffffea0000aafc00 ffff88002abf1630 000000000000000a
> 
> Call Trace:
>  [<ffffffff81764e3e>] __asan_report_load4_noabort+0x3e/0x40
> mm/kasan/report.c:294
>  [<     inline     >] debug_spin_lock_before kernel/locking/spinlock_debug.c:83
>  [<ffffffff814663d1>] do_raw_spin_lock+0x281/0x2b0
> kernel/locking/spinlock_debug.c:135
>  [<     inline     >] __raw_spin_lock_irqsave
> include/linux/spinlock_api_smp.h:119
>  [<ffffffff86652b67>] _raw_spin_lock_irqsave+0xa7/0xd0
> kernel/locking/spinlock.c:159
>  [<ffffffff852bb6a7>] snd_midi_event_decode+0x1f7/0x5c0
> sound/core/seq/seq_midi_event.c:391
>  [<     inline     >] snd_virmidi_dev_receive_event
> sound/core/seq/seq_virmidi.c:95
>  [<ffffffff852ce754>] snd_virmidi_event_input+0x214/0x310
> sound/core/seq/seq_virmidi.c:133
>  [<ffffffff852a7934>]
> snd_seq_deliver_single_event.constprop.11+0x3f4/0x740
> sound/core/seq/seq_clientmgr.c:634
>  [<ffffffff852a7da2>] snd_seq_deliver_event+0x122/0x800
> sound/core/seq/seq_clientmgr.c:828
>  [<ffffffff852a9186>] snd_seq_kernel_client_dispatch+0x126/0x170
> sound/core/seq/seq_clientmgr.c:2401
>  [<     inline     >] snd_seq_oss_dispatch
> sound/core/seq/oss/seq_oss_device.h:151
>  [<ffffffff852ca423>] snd_seq_oss_midi_reset+0x303/0x4a0
> sound/core/seq/oss/seq_oss_midi.c:481
>  [<ffffffff852bd490>] snd_seq_oss_reset+0x130/0x260
> sound/core/seq/oss/seq_oss_init.c:469
>  [<ffffffff852bd631>] snd_seq_oss_release+0x71/0x130
> sound/core/seq/oss/seq_oss_init.c:425
>  [<ffffffff852bbcca>] odev_release+0x5a/0x80 sound/core/seq/oss/seq_oss.c:155
>  [<ffffffff817c0156>] __fput+0x236/0x780 fs/file_table.c:208
>  [<ffffffff817c0725>] ____fput+0x15/0x20 fs/file_table.c:244
>  [<ffffffff813b14e0>] task_work_run+0x170/0x210 kernel/task_work.c:115
>  [<     inline     >] tracehook_notify_resume include/linux/tracehook.h:191
>  [<ffffffff810066b1>] exit_to_usermode_loop+0x1d1/0x210
> arch/x86/entry/common.c:251
>  [<     inline     >] prepare_exit_to_usermode arch/x86/entry/common.c:282
>  [<ffffffff810084ea>] syscall_return_slowpath+0x2ba/0x340
> arch/x86/entry/common.c:344
>  [<ffffffff86653322>] int_ret_from_sys_call+0x25/0x9f
> arch/x86/entry/entry_64.S:281

Looks like a race at closing virmidi device.
Does the patch below fix it?


thanks,

Takashi

---
diff --git a/sound/core/seq/seq_virmidi.c b/sound/core/seq/seq_virmidi.c
index 3da2d48610b3..f71aedfb408c 100644
--- a/sound/core/seq/seq_virmidi.c
+++ b/sound/core/seq/seq_virmidi.c
@@ -254,9 +254,13 @@ static int snd_virmidi_output_open(struct snd_rawmidi_substream *substream)
  */
 static int snd_virmidi_input_close(struct snd_rawmidi_substream *substream)
 {
+	struct snd_virmidi_dev *rdev = substream->rmidi->private_data;
 	struct snd_virmidi *vmidi = substream->runtime->private_data;
-	snd_midi_event_free(vmidi->parser);
+
+	write_lock_irq(&rdev->filelist_lock);
 	list_del(&vmidi->list);
+	write_unlock_irq(&rdev->filelist_lock);
+	snd_midi_event_free(vmidi->parser);
 	substream->runtime->private_data = NULL;
 	kfree(vmidi);
 	return 0;

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

* Re: sound: use-after-free in snd_seq_deliver_single_event
  2016-02-01 10:57   ` Takashi Iwai
  (?)
@ 2016-02-01 11:12   ` Dmitry Vyukov
  2016-02-01 11:29     ` Takashi Iwai
  -1 siblings, 1 reply; 12+ messages in thread
From: Dmitry Vyukov @ 2016-02-01 11:12 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: alsa-devel, Jie Yang, Mark Brown, Jaroslav Kysela,
	Takashi Sakamoto, LKML, Alexander Potapenko, Kostya Serebryany,
	syzkaller, Sasha Levin

On Mon, Feb 1, 2016 at 11:57 AM, Takashi Iwai <tiwai@suse.de> wrote:
> On Mon, 01 Feb 2016 11:26:57 +0100,
> Dmitry Vyukov wrote:
>>
>> Hello,
>>
>> The following program triggers use-after-free in
>> snd_seq_deliver_single_event (run in a tight parallel loop):
>>
>> // autogenerated by syzkaller (http://github.com/google/syzkaller)
>> #include <pthread.h>
>> #include <stdint.h>
>> #include <string.h>
>> #include <sys/syscall.h>
>> #include <unistd.h>
>>
>> #ifndef SYS_mmap
>> #define SYS_mmap 9
>> #endif
>> #ifndef SYS_syz_open_dev
>> #define SYS_syz_open_dev 1000001
>> #endif
>> #ifndef SYS_ioctl
>> #define SYS_ioctl 16
>> #endif
>> #ifndef SYS_getpgrp
>> #define SYS_getpgrp 111
>> #endif
>> #ifndef SYS_migrate_pages
>> #define SYS_migrate_pages 256
>> #endif
>> #ifndef SYS_read
>> #define SYS_read 0
>> #endif
>> #ifndef SYS_write
>> #define SYS_write 1
>> #endif
>> #ifndef SYS_dup3
>> #define SYS_dup3 292
>> #endif
>>
>> long r[59];
>>
>> void* thr(void* arg)
>> {
>>   switch ((long)arg) {
>>   case 0:
>>     r[0] = syscall(SYS_mmap, 0x20000000ul, 0x47000ul, 0x3ul, 0x32ul,
>>                    0xfffffffffffffffful, 0x0ul);
>>     break;
>>   case 1:
>>     memcpy((void*)0x20000000,
>>            "\x2f\x64\x65\x76\x2f\x73\x65\x71\x75\x65\x6e\x63\x65\x72",
>>            14);
>>     r[2] =
>>         syscall(SYS_open, "/dev/sequencer", 0x2ul, 0, 0, 0);
>>     break;
>>   case 2:
>>     r[3] = syscall(SYS_mmap, 0x20047000ul, 0x1000ul, 0x3ul, 0x32ul,
>>                    0xfffffffffffffffful, 0x0ul);
>>     break;
>>   case 3:
>>     r[4] = syscall(SYS_ioctl, r[2], 0x540ful, 0x20047ffcul, 0, 0, 0);
>>     if (r[4] != -1)
>>       r[5] = *(uint32_t*)0x20047ffc;
>>     break;
>>   case 4:
>>     r[6] = syscall(SYS_getpgrp, r[5], 0, 0, 0, 0, 0);
>>     break;
>>   case 5:
>>     r[7] = syscall(SYS_mmap, 0x20047000ul, 0x1000ul, 0x3ul, 0x32ul,
>>                    0xfffffffffffffffful, 0x0ul);
>>     break;
>>   case 6:
>>     r[8] = syscall(SYS_mmap, 0x20047000ul, 0x1000ul, 0x3ul, 0x32ul,
>>                    0xfffffffffffffffful, 0x0ul);
>>     break;
>>   case 7:
>>     *(uint64_t*)0x20047000 = (uint64_t)0x5;
>>     *(uint64_t*)0x20047000 = (uint64_t)0xfffffffffffffff8;
>>     r[11] = syscall(SYS_migrate_pages, r[6], 0xfful, 0x20047000ul,
>>                     0x20047000ul, 0, 0);
>>     break;
>>   case 8:
>>     memcpy((void*)0x20043684,
>>            "\x2f\x64\x65\x76\x2f\x64\x6d\x6d\x69\x64\x69\x23", 12);
>>     r[13] = syscall(SYS_open, "/dev/dmmidi2", 0x8a000ul, 0, 0, 0);
>>     break;
>>   case 9:
>>     r[14] = syscall(SYS_read, r[13], 0x20045000ul, 0x36ul, 0, 0, 0);
>>     break;
>>   case 10:
>>     *(uint8_t*)0x2000042c = (uint8_t)0x0;
>>     *(uint8_t*)0x2000042d = (uint8_t)0x9cf;
>>     *(uint8_t*)0x2000042e = (uint8_t)0x9;
>>     *(uint8_t*)0x2000042f = (uint8_t)0x0;
>>     *(uint64_t*)0x20000444 = (uint64_t)0x0;
>>     *(uint64_t*)0x2000044c = (uint64_t)0x989680;
>>     *(uint8_t*)0x20000454 = (uint8_t)0x2;
>>     *(uint8_t*)0x20000455 = (uint8_t)0x100000000;
>>     *(uint8_t*)0x20000456 = (uint8_t)0x8d;
>>     *(uint8_t*)0x20000457 = (uint8_t)0x7;
>>     *(uint8_t*)0x20000464 = (uint8_t)0x51b2;
>>     *(uint8_t*)0x20000465 = (uint8_t)0xfffffffffffff15a;
>>     *(uint8_t*)0x20000466 = (uint8_t)0x4;
>>     *(uint8_t*)0x20000467 = (uint8_t)0x4;
>>     *(uint32_t*)0x20000468 = (uint32_t)0xffffffff;
>>     *(uint8_t*)0x2000046c = (uint8_t)0x5;
>>     *(uint8_t*)0x2000046d = (uint8_t)0xfffffffffffffff8;
>>     *(uint8_t*)0x2000046e = (uint8_t)0x401;
>>     *(uint8_t*)0x2000046f = (uint8_t)0x5;
>>     *(uint64_t*)0x20000484 = (uint64_t)0x4;
>>     *(uint64_t*)0x2000048c = (uint64_t)0x0;
>>     *(uint8_t*)0x20000494 = (uint8_t)0xc2c1;
>>     *(uint8_t*)0x20000495 = (uint8_t)0x1;
>>     *(uint8_t*)0x20000496 = (uint8_t)0x6b;
>>     *(uint8_t*)0x20000497 = (uint8_t)0x6;
>>     *(uint8_t*)0x200004a0 = (uint8_t)0xfffffffffffffffa;
>>     *(uint8_t*)0x200004a1 = (uint8_t)0x81;
>>     *(uint8_t*)0x200004a2 = (uint8_t)0x0;
>>     *(uint8_t*)0x200004a3 = (uint8_t)0xa6;
>>     *(uint8_t*)0x200004a4 = (uint8_t)0x400;
>>     *(uint8_t*)0x200004a5 = (uint8_t)0x3ff;
>>     *(uint8_t*)0x200004a6 = (uint8_t)0x8;
>>     *(uint8_t*)0x200004a7 = (uint8_t)0x3;
>>     *(uint64_t*)0x200004bc = (uint64_t)0x0;
>>     *(uint64_t*)0x200004c4 = (uint64_t)0x0;
>>     *(uint8_t*)0x200004cc = (uint8_t)0x9e;
>>     *(uint8_t*)0x200004cd = (uint8_t)0x7ff;
>>     *(uint8_t*)0x200004ce = (uint8_t)0x7fff;
>>     *(uint8_t*)0x200004cf = (uint8_t)0x9;
>>     *(uint8_t*)0x200004e0 = (uint8_t)0xa;
>>     *(uint32_t*)0x200004e4 = (uint32_t)0x6;
>>     *(uint32_t*)0x200004e8 = (uint32_t)0xffffffff;
>>     r[57] = syscall(SYS_write, r[2], 0x2000042cul, 0x78ul, 0, 0, 0);
>>     break;
>>   case 11:
>>     r[58] = syscall(SYS_dup3, r[13], r[2], 0x80000ul, 0, 0, 0);
>>     break;
>>   }
>>   return 0;
>> }
>>
>> int main()
>> {
>>   long i;
>>   pthread_t th[12];
>>
>>   memset(r, -1, sizeof(r));
>>   for (i = 0; i < 12; i++) {
>>     pthread_create(&th[i], 0, thr, (void*)i);
>>     usleep(10000);
>>   }
>>   for (i = 0; i < 12; i++) {
>>     pthread_create(&th[i], 0, thr, (void*)i);
>>     if (i % 2 == 0)
>>       usleep(10000);
>>   }
>>   usleep(100000);
>>   return 0;
>> }
>>
>>
>> ==================================================================
>> BUG: KASAN: use-after-free in do_raw_spin_lock+0x281/0x2b0 at addr
>> ffff88002abf1654
>> Read of size 4 by task syz-executor/13987
>> =============================================================================
>> BUG kmalloc-96 (Not tainted): kasan: bad access detected
>> -----------------------------------------------------------------------------
>>
>> INFO: Allocated in snd_midi_event_new+0x73/0x200 age=10 cpu=0 pid=13996
>> [<      none      >] ___slab_alloc+0x564/0x5b0 mm/slub.c:2470
>> [<      none      >] __slab_alloc+0x66/0xc0 mm/slub.c:2499
>> [<     inline     >] slab_alloc_node mm/slub.c:2562
>> [<     inline     >] slab_alloc mm/slub.c:2604
>> [<      none      >] kmem_cache_alloc_trace+0x25c/0x300 mm/slub.c:2621
>> [<     inline     >] kmalloc include/linux/slab.h:463
>> [<     inline     >] kzalloc include/linux/slab.h:607
>> [<      none      >] snd_midi_event_new+0x73/0x200
>> sound/core/seq/seq_midi_event.c:120
>> [<      none      >] snd_virmidi_input_open+0xfd/0x380
>> sound/core/seq/seq_virmidi.c:214
>> [<      none      >] open_substream+0x3eb/0x780 sound/core/rawmidi.c:269
>> [<      none      >] rawmidi_open_priv+0x144/0x300 sound/core/rawmidi.c:312
>> [<      none      >] snd_rawmidi_open+0x3fb/0xa90 sound/core/rawmidi.c:416
>> [<      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:3254
>> [<      none      >] path_openat+0xde9/0x5e30 fs/namei.c:3386
>> [<      none      >] do_filp_open+0x18e/0x250 fs/namei.c:3421
>> [<      none      >] do_sys_open+0x1fc/0x420 fs/open.c:1022
>> [<     inline     >] SYSC_open fs/open.c:1040
>> [<      none      >] SyS_open+0x2d/0x40 fs/open.c:1035
>>
>> INFO: Freed in snd_midi_event_free+0x43/0x60 age=17 cpu=0 pid=13996
>> [<      none      >] __slab_free+0x1fc/0x320 mm/slub.c:2680
>> [<     inline     >] slab_free mm/slub.c:2835
>> [<      none      >] kfree+0x2ac/0x2c0 mm/slub.c:3664
>> [<      none      >] snd_midi_event_free+0x43/0x60
>> sound/core/seq/seq_midi_event.c:142
>> [<      none      >] snd_virmidi_input_close+0x87/0x120
>> sound/core/seq/seq_virmidi.c:261
>> [<      none      >] close_substream.part.14+0xda/0x540 sound/core/rawmidi.c:486
>> [<     inline     >] close_substream sound/core/rawmidi.c:514
>> [<      none      >] rawmidi_release_priv+0x1e4/0x250 sound/core/rawmidi.c:504
>> [<      none      >] snd_rawmidi_release+0x62/0xf0 sound/core/rawmidi.c:539
>> [<      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+0x8b5/0x2cb0 kernel/exit.c:748
>> [<      none      >] do_group_exit+0x108/0x330 kernel/exit.c:878
>> [<      none      >] get_signal+0x5e4/0x14f0 kernel/signal.c:2307
>> [<      none      >] do_signal+0x83/0x1c90 arch/x86/kernel/signal.c:712
>> [<      none      >] exit_to_usermode_loop+0x1a5/0x210
>> arch/x86/entry/common.c:247
>> [<     inline     >] prepare_exit_to_usermode arch/x86/entry/common.c:282
>> [<      none      >] syscall_return_slowpath+0x2ba/0x340
>> arch/x86/entry/common.c:344
>>
>> INFO: Slab 0xffffea0000aafc00 objects=28 used=11 fp=0xffff88002abf0470
>> flags=0x1fffc0000004080
>> INFO: Object 0xffff88002abf1630 @offset=5680 fp=0xffff88002abf06a8
>> CPU: 1 PID: 13987 Comm: syz-executor Tainted: G    B           4.5.0-rc1+ #305
>> Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
>>  00000000ffffffff ffff880061aaf7d8 ffffffff82be11ad ffff88003e804900
>>  ffff88002abf1630 ffff88002abf0000 ffff880061aaf808 ffffffff8175b454
>>  ffff88003e804900 ffffea0000aafc00 ffff88002abf1630 000000000000000a
>>
>> Call Trace:
>>  [<ffffffff81764e3e>] __asan_report_load4_noabort+0x3e/0x40
>> mm/kasan/report.c:294
>>  [<     inline     >] debug_spin_lock_before kernel/locking/spinlock_debug.c:83
>>  [<ffffffff814663d1>] do_raw_spin_lock+0x281/0x2b0
>> kernel/locking/spinlock_debug.c:135
>>  [<     inline     >] __raw_spin_lock_irqsave
>> include/linux/spinlock_api_smp.h:119
>>  [<ffffffff86652b67>] _raw_spin_lock_irqsave+0xa7/0xd0
>> kernel/locking/spinlock.c:159
>>  [<ffffffff852bb6a7>] snd_midi_event_decode+0x1f7/0x5c0
>> sound/core/seq/seq_midi_event.c:391
>>  [<     inline     >] snd_virmidi_dev_receive_event
>> sound/core/seq/seq_virmidi.c:95
>>  [<ffffffff852ce754>] snd_virmidi_event_input+0x214/0x310
>> sound/core/seq/seq_virmidi.c:133
>>  [<ffffffff852a7934>]
>> snd_seq_deliver_single_event.constprop.11+0x3f4/0x740
>> sound/core/seq/seq_clientmgr.c:634
>>  [<ffffffff852a7da2>] snd_seq_deliver_event+0x122/0x800
>> sound/core/seq/seq_clientmgr.c:828
>>  [<ffffffff852a9186>] snd_seq_kernel_client_dispatch+0x126/0x170
>> sound/core/seq/seq_clientmgr.c:2401
>>  [<     inline     >] snd_seq_oss_dispatch
>> sound/core/seq/oss/seq_oss_device.h:151
>>  [<ffffffff852ca423>] snd_seq_oss_midi_reset+0x303/0x4a0
>> sound/core/seq/oss/seq_oss_midi.c:481
>>  [<ffffffff852bd490>] snd_seq_oss_reset+0x130/0x260
>> sound/core/seq/oss/seq_oss_init.c:469
>>  [<ffffffff852bd631>] snd_seq_oss_release+0x71/0x130
>> sound/core/seq/oss/seq_oss_init.c:425
>>  [<ffffffff852bbcca>] odev_release+0x5a/0x80 sound/core/seq/oss/seq_oss.c:155
>>  [<ffffffff817c0156>] __fput+0x236/0x780 fs/file_table.c:208
>>  [<ffffffff817c0725>] ____fput+0x15/0x20 fs/file_table.c:244
>>  [<ffffffff813b14e0>] task_work_run+0x170/0x210 kernel/task_work.c:115
>>  [<     inline     >] tracehook_notify_resume include/linux/tracehook.h:191
>>  [<ffffffff810066b1>] exit_to_usermode_loop+0x1d1/0x210
>> arch/x86/entry/common.c:251
>>  [<     inline     >] prepare_exit_to_usermode arch/x86/entry/common.c:282
>>  [<ffffffff810084ea>] syscall_return_slowpath+0x2ba/0x340
>> arch/x86/entry/common.c:344
>>  [<ffffffff86653322>] int_ret_from_sys_call+0x25/0x9f
>> arch/x86/entry/entry_64.S:281
>
> Looks like a race at closing virmidi device.
> Does the patch below fix it?


This seems to help.

> thanks,
>
> Takashi
>
> ---
> diff --git a/sound/core/seq/seq_virmidi.c b/sound/core/seq/seq_virmidi.c
> index 3da2d48610b3..f71aedfb408c 100644
> --- a/sound/core/seq/seq_virmidi.c
> +++ b/sound/core/seq/seq_virmidi.c
> @@ -254,9 +254,13 @@ static int snd_virmidi_output_open(struct snd_rawmidi_substream *substream)
>   */
>  static int snd_virmidi_input_close(struct snd_rawmidi_substream *substream)
>  {
> +       struct snd_virmidi_dev *rdev = substream->rmidi->private_data;
>         struct snd_virmidi *vmidi = substream->runtime->private_data;
> -       snd_midi_event_free(vmidi->parser);
> +
> +       write_lock_irq(&rdev->filelist_lock);
>         list_del(&vmidi->list);
> +       write_unlock_irq(&rdev->filelist_lock);
> +       snd_midi_event_free(vmidi->parser);
>         substream->runtime->private_data = NULL;
>         kfree(vmidi);
>         return 0;

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

* Re: sound: use-after-free in snd_seq_deliver_single_event
  2016-02-01 10:26 sound: use-after-free in snd_seq_deliver_single_event Dmitry Vyukov
  2016-02-01 10:57   ` Takashi Iwai
@ 2016-02-01 11:16 ` Mark Brown
  2016-02-01 11:22   ` Dmitry Vyukov
  1 sibling, 1 reply; 12+ messages in thread
From: Mark Brown @ 2016-02-01 11:16 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Jaroslav Kysela, Takashi Iwai, Jie Yang, Takashi Sakamoto,
	alsa-devel, LKML, syzkaller, Kostya Serebryany,
	Alexander Potapenko, Sasha Levin

[-- Attachment #1: Type: text/plain, Size: 505 bytes --]

On Mon, Feb 01, 2016 at 11:26:57AM +0100, Dmitry Vyukov wrote:

> The following program triggers use-after-free in
> snd_seq_deliver_single_event (run in a tight parallel loop):

I'm not sure how you're working out who to send these to but all these
reports you've been sending have been for ALSA core which I rarely look
at closely, I mostly look at ASoC.  Might be worth looking into so
people don't start zoning out things that look like misdirected stuff
(I just noticed myself doing that with this).

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: sound: use-after-free in snd_seq_deliver_single_event
  2016-02-01 11:16 ` Mark Brown
@ 2016-02-01 11:22   ` Dmitry Vyukov
  2016-02-01 12:03     ` [alsa-devel] " Takashi Iwai
  2016-02-01 12:20     ` Mark Brown
  0 siblings, 2 replies; 12+ messages in thread
From: Dmitry Vyukov @ 2016-02-01 11:22 UTC (permalink / raw)
  To: Mark Brown
  Cc: Jaroslav Kysela, Takashi Iwai, Jie Yang, Takashi Sakamoto,
	alsa-devel, LKML, syzkaller, Kostya Serebryany,
	Alexander Potapenko, Sasha Levin

On Mon, Feb 1, 2016 at 12:16 PM, Mark Brown <broonie@kernel.org> wrote:
> On Mon, Feb 01, 2016 at 11:26:57AM +0100, Dmitry Vyukov wrote:
>
>> The following program triggers use-after-free in
>> snd_seq_deliver_single_event (run in a tight parallel loop):
>
> I'm not sure how you're working out who to send these to but all these
> reports you've been sending have been for ALSA core which I rarely look
> at closely, I mostly look at ASoC.  Might be worth looking into so
> people don't start zoning out things that look like misdirected stuff
> (I just noticed myself doing that with this).


I am using scripts/get_maintainer.pl -f  the involved files. If it
produces wrong results, please update MAINTAINERS.

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

* Re: sound: use-after-free in snd_seq_deliver_single_event
  2016-02-01 11:12   ` Dmitry Vyukov
@ 2016-02-01 11:29     ` Takashi Iwai
  0 siblings, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2016-02-01 11:29 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: alsa-devel, Jie Yang, Mark Brown, Jaroslav Kysela,
	Takashi Sakamoto, LKML, Alexander Potapenko, Kostya Serebryany,
	syzkaller, Sasha Levin

On Mon, 01 Feb 2016 12:12:21 +0100,
Dmitry Vyukov wrote:
> 
> > Looks like a race at closing virmidi device.
> > Does the patch below fix it?
> 
> 
> This seems to help.

Thanks for a quick test.  FWIW, below is the final patch I'm going to
queue.


Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: seq: Fix race at closing in virmidi driver

The virmidi driver has an open race at closing its assigned rawmidi
device, and this may lead to use-after-free in
snd_seq_deliver_single_event().

Plug the hole by properly protecting the linked list deletion and
calling in the right order in snd_virmidi_input_close().

BugLink: http://lkml.kernel.org/r/CACT4Y+Zd66+w12fNN85-425cVQT=K23kWbhnCEcMB8s3us-Frw@mail.gmail.com
Reported-by: Dmitry Vyukov <dvyukov@google.com>
Tested-by: Dmitry Vyukov <dvyukov@google.com>
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 sound/core/seq/seq_virmidi.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/sound/core/seq/seq_virmidi.c b/sound/core/seq/seq_virmidi.c
index 3da2d48610b3..f71aedfb408c 100644
--- a/sound/core/seq/seq_virmidi.c
+++ b/sound/core/seq/seq_virmidi.c
@@ -254,9 +254,13 @@ static int snd_virmidi_output_open(struct snd_rawmidi_substream *substream)
  */
 static int snd_virmidi_input_close(struct snd_rawmidi_substream *substream)
 {
+	struct snd_virmidi_dev *rdev = substream->rmidi->private_data;
 	struct snd_virmidi *vmidi = substream->runtime->private_data;
-	snd_midi_event_free(vmidi->parser);
+
+	write_lock_irq(&rdev->filelist_lock);
 	list_del(&vmidi->list);
+	write_unlock_irq(&rdev->filelist_lock);
+	snd_midi_event_free(vmidi->parser);
 	substream->runtime->private_data = NULL;
 	kfree(vmidi);
 	return 0;
-- 
2.7.0

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

* Re: [alsa-devel] sound: use-after-free in snd_seq_deliver_single_event
  2016-02-01 11:22   ` Dmitry Vyukov
@ 2016-02-01 12:03     ` Takashi Iwai
  2016-02-01 12:20     ` Mark Brown
  1 sibling, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2016-02-01 12:03 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Mark Brown, alsa-devel, LKML, Takashi Sakamoto,
	Kostya Serebryany, syzkaller, Alexander Potapenko, Sasha Levin

On Mon, 01 Feb 2016 12:22:18 +0100,
Dmitry Vyukov wrote:
> 
> On Mon, Feb 1, 2016 at 12:16 PM, Mark Brown <broonie@kernel.org> wrote:
> > On Mon, Feb 01, 2016 at 11:26:57AM +0100, Dmitry Vyukov wrote:
> >
> >> The following program triggers use-after-free in
> >> snd_seq_deliver_single_event (run in a tight parallel loop):
> >
> > I'm not sure how you're working out who to send these to but all these
> > reports you've been sending have been for ALSA core which I rarely look
> > at closely, I mostly look at ASoC.  Might be worth looking into so
> > people don't start zoning out things that look like misdirected stuff
> > (I just noticed myself doing that with this).
> 
> 
> I am using scripts/get_maintainer.pl -f  the involved files. If it
> produces wrong results, please update MAINTAINERS.

Well, MAINTAINERS file points that Mark is responsible for
sound/soc/*, but not for sound/* in general.


Takashi

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

* Re: sound: use-after-free in snd_seq_deliver_single_event
  2016-02-01 11:22   ` Dmitry Vyukov
  2016-02-01 12:03     ` [alsa-devel] " Takashi Iwai
@ 2016-02-01 12:20     ` Mark Brown
  2016-02-01 13:34       ` Takashi Sakamoto
  2016-02-01 13:43       ` Takashi Sakamoto
  1 sibling, 2 replies; 12+ messages in thread
From: Mark Brown @ 2016-02-01 12:20 UTC (permalink / raw)
  To: Dmitry Vyukov
  Cc: Jaroslav Kysela, Takashi Iwai, Jie Yang, Takashi Sakamoto,
	alsa-devel, LKML, syzkaller, Kostya Serebryany,
	Alexander Potapenko, Sasha Levin

[-- Attachment #1: Type: text/plain, Size: 1187 bytes --]

On Mon, Feb 01, 2016 at 12:22:18PM +0100, Dmitry Vyukov wrote:
> On Mon, Feb 1, 2016 at 12:16 PM, Mark Brown <broonie@kernel.org> wrote:

> > I'm not sure how you're working out who to send these to but all these
> > reports you've been sending have been for ALSA core which I rarely look
> > at closely, I mostly look at ASoC.  Might be worth looking into so
> > people don't start zoning out things that look like misdirected stuff
> > (I just noticed myself doing that with this).

> I am using scripts/get_maintainer.pl -f  the involved files. If it
> produces wrong results, please update MAINTAINERS.

You should never rely on the output of get_maintainers without review,
it's prone to both false positives and false negatives.  Given that
Sakamoto-san does not seem to appear in MAINTAINERS at all and you've
not managed to find Liam who's a comaintainer for all the ASoC stuff I
suspect you're doing this with --git enabled which is especially prone
to false positives since it tends to identify people who are just doing
global cleanup work and aren't particularly interested in a given file.
As far as I can tell this is how I'm getting pulled in too rather than
MAINTAINERS.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

* Re: sound: use-after-free in snd_seq_deliver_single_event
  2016-02-01 12:20     ` Mark Brown
@ 2016-02-01 13:34       ` Takashi Sakamoto
  2016-02-02 19:26         ` Mark Brown
  2016-02-01 13:43       ` Takashi Sakamoto
  1 sibling, 1 reply; 12+ messages in thread
From: Takashi Sakamoto @ 2016-02-01 13:34 UTC (permalink / raw)
  To: Mark Brown, Dmitry Vyukov
  Cc: alsa-devel, Takashi Iwai, LKML, Kostya Serebryany, syzkaller,
	Alexander Potapenko, Sasha Levin

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Hi,

On Feb 01 2016 21:20, Mark Brown wrote:
> On Mon, Feb 01, 2016 at 12:22:18PM +0100, Dmitry Vyukov wrote:
>> On Mon, Feb 1, 2016 at 12:16 PM, Mark Brown <broonie@kernel.org>
>> wrote:
> 
>>> I'm not sure how you're working out who to send these to but
>>> all these reports you've been sending have been for ALSA core
>>> which I rarely look at closely, I mostly look at ASoC.  Might
>>> be worth looking into so people don't start zoning out things
>>> that look like misdirected stuff (I just noticed myself doing
>>> that with this).
> 
>> I am using scripts/get_maintainer.pl -f  the involved files. If
>> it produces wrong results, please update MAINTAINERS.
> 
> You should never rely on the output of get_maintainers without
> review, it's prone to both false positives and false negatives.
> Given that Sakamoto-san does not seem to appear in MAINTAINERS at
> all and you've not managed to find Liam who's a comaintainer for
> all the ASoC stuff I suspect you're doing this with --git enabled
> which is especially prone to false positives since it tends to
> identify people who are just doing global cleanup work and aren't
> particularly interested in a given file. As far as I can tell this
> is how I'm getting pulled in too rather than MAINTAINERS.

Aha. That's the reason I receive these messages. I've wondering why I
receive them.

Well, I don't mind to receive them because it's a good information for
me to get current state of ALSA core functionality. If I had some
spare time, I was also going to fix it. But currently I have little
time for it due to my development for my minor hardwares...

(Actually, Iwai-san posted patches to fix them at incredibly pace. I
can't follow his pace, because I'm working for ALSA in my free time.)


Regards

Takashi Sakamoto
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAEBCAAGBQJWr17cAAoJENbkvsBXhK8aQAgH/0Tz3LSqfNVVGaXIpYJDS7NF
MowxJGAP9SuE9loduWh3afsX1wllnVLpqtsAe1jIehKWWOGNxQjYF2BnLw4ILBaC
tIv5KUN+hYOXWx91obYyRbxZ/r4oJDA2TofkIul2cHhu7td5QnwFiCRbLgPzilns
dXQ6W3hBKZ0luM+knDDJgHn28hkLDIsmBdhrAKv3RsdDsvM41eeJMEBBbwCUkDq5
akqBpP95rA1rCo61nVn/UGydqcbOXMXcDvDQa+nAZcLjKZixysLcyK1gn4DMN6dg
SCSMtnuLO3rR49B46ea81m9qjy0iJX43cmgXTzOk0CSax3oqhPUerWnaLxAD0ps=
=dt+s
-----END PGP SIGNATURE-----

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

* Re: sound: use-after-free in snd_seq_deliver_single_event
  2016-02-01 12:20     ` Mark Brown
  2016-02-01 13:34       ` Takashi Sakamoto
@ 2016-02-01 13:43       ` Takashi Sakamoto
  1 sibling, 0 replies; 12+ messages in thread
From: Takashi Sakamoto @ 2016-02-01 13:43 UTC (permalink / raw)
  To: Mark Brown, Dmitry Vyukov
  Cc: alsa-devel, Takashi Iwai, LKML, Kostya Serebryany, syzkaller,
	Alexander Potapenko, Sasha Levin

On Feb 01 2016 21:20, Mark Brown wrote:
> On Mon, Feb 01, 2016 at 12:22:18PM +0100, Dmitry Vyukov wrote:
>> On Mon, Feb 1, 2016 at 12:16 PM, Mark Brown <broonie@kernel.org> wrote:
> 
>>> I'm not sure how you're working out who to send these to but all these
>>> reports you've been sending have been for ALSA core which I rarely look
>>> at closely, I mostly look at ASoC.  Might be worth looking into so
>>> people don't start zoning out things that look like misdirected stuff
>>> (I just noticed myself doing that with this).
> 
>> I am using scripts/get_maintainer.pl -f  the involved files. If it
>> produces wrong results, please update MAINTAINERS.
> 
> You should never rely on the output of get_maintainers without review,
> it's prone to both false positives and false negatives.  Given that
> Sakamoto-san does not seem to appear in MAINTAINERS at all and you've
> not managed to find Liam who's a comaintainer for all the ASoC stuff I
> suspect you're doing this with --git enabled which is especially prone
> to false positives since it tends to identify people who are just doing
> global cleanup work and aren't particularly interested in a given file.
> As far as I can tell this is how I'm getting pulled in too rather than
> MAINTAINERS.

Aha. That's the reason I receive these messages. I've wondering why I
receive them.

Well, I don't mind to receive them because it's a good information for
me to get current state of ALSA core functionality. If I had some
spare time, I was also going to fix it. But currently I have little
time for it due to my development for my minor hardwares...

(Actually, Iwai-san posted patches to fix them at incredibly pace. I
can't follow his pace, because I'm working for ALSA in my free time.)


Regards

Takashi Sakamoto

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

* Re: sound: use-after-free in snd_seq_deliver_single_event
  2016-02-01 13:34       ` Takashi Sakamoto
@ 2016-02-02 19:26         ` Mark Brown
  0 siblings, 0 replies; 12+ messages in thread
From: Mark Brown @ 2016-02-02 19:26 UTC (permalink / raw)
  To: Takashi Sakamoto
  Cc: Dmitry Vyukov, alsa-devel, Takashi Iwai, LKML, Kostya Serebryany,
	syzkaller, Alexander Potapenko, Sasha Levin

[-- Attachment #1: Type: text/plain, Size: 575 bytes --]

On Mon, Feb 01, 2016 at 10:34:23PM +0900, Takashi Sakamoto wrote:

> Well, I don't mind to receive them because it's a good information for
> me to get current state of ALSA core functionality. If I had some
> spare time, I was also going to fix it. But currently I have little
> time for it due to my development for my minor hardwares...

Yeah, my concern is that I'm getting to the point where I mentally tag
them as irrelevant and delete them unread (the subject lines don't help
here).  I've had quite a few and none of them have been for code I even
recall working on.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

end of thread, other threads:[~2016-02-02 19:26 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-01 10:26 sound: use-after-free in snd_seq_deliver_single_event Dmitry Vyukov
2016-02-01 10:57 ` Takashi Iwai
2016-02-01 10:57   ` Takashi Iwai
2016-02-01 11:12   ` Dmitry Vyukov
2016-02-01 11:29     ` Takashi Iwai
2016-02-01 11:16 ` Mark Brown
2016-02-01 11:22   ` Dmitry Vyukov
2016-02-01 12:03     ` [alsa-devel] " Takashi Iwai
2016-02-01 12:20     ` Mark Brown
2016-02-01 13:34       ` Takashi Sakamoto
2016-02-02 19:26         ` Mark Brown
2016-02-01 13:43       ` Takashi Sakamoto

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.