usb/midi: use-after-free in snd_rawmidi_dev_seq_free

[Andrey Konovalov]

Hi!

While working on adding USB fuzzing support to syzkaller, I got the
following crash.

On commit 81a84ad3cb5711cec79f4dd53a4ce026b092c432 (4.13+).

My .config is attached. I don't have a reproducer for this bug.

==================================================================
BUG: KASAN: use-after-free in snd_rawmidi_dev_seq_free+0x69/0x70
sound/core/rawmidi.c:1617
Write of size 8 at addr ffff88006c611d90 by task kworker/2:1/567

CPU: 2 PID: 567 Comm: kworker/2:1 Not tainted 4.13.0+ #29
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Workqueue: events autoload_drivers
Call Trace:
__dump_stack lib/dump_stack.c:16 [inline]
dump_stack+0x192/0x22c lib/dump_stack.c:52
print_address_description+0x78/0x280 mm/kasan/report.c:252
kasan_report_error mm/kasan/report.c:351 [inline]
kasan_report+0x230/0x340 mm/kasan/report.c:409
__asan_report_store8_noabort+0x1c/0x20 mm/kasan/report.c:435
snd_rawmidi_dev_seq_free+0x69/0x70 sound/core/rawmidi.c:1617
snd_seq_dev_release+0x4f/0x70 sound/core/seq_device.c:192
device_release+0x13f/0x210 drivers/base/core.c:814
kobject_cleanup lib/kobject.c:648 [inline]
kobject_release lib/kobject.c:677 [inline]
kref_put include/linux/kref.h:70 [inline]
kobject_put+0x145/0x240 lib/kobject.c:694
put_device+0x25/0x30 drivers/base/core.c:1799
klist_devices_put+0x36/0x40 drivers/base/bus.c:827
klist_next+0x264/0x4a0 lib/klist.c:403
next_device drivers/base/bus.c:270 [inline]
bus_for_each_dev+0x17e/0x210 drivers/base/bus.c:312
autoload_drivers+0x3b/0x50 sound/core/seq_device.c:117
process_one_work+0x9fb/0x1570 kernel/workqueue.c:2097
worker_thread+0x1e4/0x1350 kernel/workqueue.c:2231
kthread+0x324/0x3f0 kernel/kthread.c:231
ret_from_fork+0x25/0x30 arch/x86/entry/entry_64.S:425

Allocated by task 567:
save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
kasan_kmalloc+0xad/0xe0 mm/kasan/kasan.c:551
kmem_cache_alloc_trace+0x66/0x170 mm/slub.c:2739
kmalloc include/linux/slab.h:493 [inline]
kzalloc include/linux/slab.h:666 [inline]
snd_rawmidi_new+0x111/0x500 sound/core/rawmidi.c:1535
snd_usbmidi_create_rawmidi sound/usb/midi.c:2221 [inline]
__snd_usbmidi_create+0x5cb/0x2700 sound/usb/midi.c:2450
snd_usbmidi_create sound/usb/midi.h:53 [inline]
create_any_midi_quirk+0x54/0x70 sound/usb/quirks.c:103
snd_usb_create_quirk+0x92/0x100 sound/usb/quirks.c:561
usb_audio_probe+0xf98/0x2aa0 sound/usb/card.c:598
usb_probe_interface+0x351/0x8d0 drivers/usb/core/driver.c:361
really_probe drivers/base/dd.c:385 [inline]
driver_probe_device+0x610/0xa00 drivers/base/dd.c:529
__device_attach_driver+0x230/0x290 drivers/base/dd.c:625
bus_for_each_drv+0x15e/0x210 drivers/base/bus.c:463
__device_attach+0x269/0x3c0 drivers/base/dd.c:682
device_initial_probe+0x1f/0x30 drivers/base/dd.c:729
bus_probe_device+0x1da/0x280 drivers/base/bus.c:523
device_add+0xcf9/0x1640 drivers/base/core.c:1703
usb_set_configuration+0x1064/0x1890 drivers/usb/core/message.c:1932
generic_probe+0x73/0xe0 drivers/usb/core/generic.c:174
usb_probe_device+0xaf/0xe0 drivers/usb/core/driver.c:266
really_probe drivers/base/dd.c:385 [inline]
driver_probe_device+0x610/0xa00 drivers/base/dd.c:529
__device_attach_driver+0x230/0x290 drivers/base/dd.c:625
bus_for_each_drv+0x15e/0x210 drivers/base/bus.c:463
__device_attach+0x269/0x3c0 drivers/base/dd.c:682
device_initial_probe+0x1f/0x30 drivers/base/dd.c:729
bus_probe_device+0x1da/0x280 drivers/base/bus.c:523
device_add+0xcf9/0x1640 drivers/base/core.c:1703
usb_new_device+0x91d/0x17a0 drivers/usb/core/hub.c:2457
hub_port_connect drivers/usb/core/hub.c:4890 [inline]
hub_port_connect_change drivers/usb/core/hub.c:4996 [inline]
port_event drivers/usb/core/hub.c:5102 [inline]
hub_event+0x261c/0x3960 drivers/usb/core/hub.c:5186
process_one_work+0x9fb/0x1570 kernel/workqueue.c:2097
worker_thread+0x1e4/0x1350 kernel/workqueue.c:2231
kthread+0x324/0x3f0 kernel/kthread.c:231
ret_from_fork+0x25/0x30 arch/x86/entry/entry_64.S:425

Freed by task 1454:
save_stack_trace+0x1b/0x20 arch/x86/kernel/stacktrace.c:59
save_stack+0x43/0xd0 mm/kasan/kasan.c:447
set_track mm/kasan/kasan.c:459 [inline]
kasan_slab_free+0x72/0xc0 mm/kasan/kasan.c:524
slab_free_hook mm/slub.c:1357 [inline]
slab_free_freelist_hook mm/slub.c:1379 [inline]
slab_free mm/slub.c:2955 [inline]
kfree+0x96/0x1a0 mm/slub.c:3883
release_rawmidi_device+0x1e/0x30 sound/core/rawmidi.c:1502
device_release+0x13f/0x210 drivers/base/core.c:814
kobject_cleanup lib/kobject.c:648 [inline]
kobject_release lib/kobject.c:677 [inline]
kref_put include/linux/kref.h:70 [inline]
kobject_put+0x145/0x240 lib/kobject.c:694
put_device+0x25/0x30 drivers/base/core.c:1799
snd_rawmidi_free.part.15+0x14b/0x190 sound/core/rawmidi.c:1603
snd_rawmidi_free sound/core/rawmidi.c:1589 [inline]
snd_rawmidi_dev_free+0x43/0x60 sound/core/rawmidi.c:1610
__snd_device_free+0x1e5/0x380 sound/core/device.c:91
snd_device_free_all+0x8f/0xe0 sound/core/device.c:244
snd_card_do_free sound/core/init.c:463 [inline]
release_card_device+0x26/0x150 sound/core/init.c:181
device_release+0x13f/0x210 drivers/base/core.c:814
kobject_cleanup lib/kobject.c:648 [inline]
kobject_release lib/kobject.c:677 [inline]
kref_put include/linux/kref.h:70 [inline]
kobject_put+0x145/0x240 lib/kobject.c:694
put_device+0x25/0x30 drivers/base/core.c:1799
snd_card_free_when_closed+0x34/0x50 sound/core/init.c:491
usb_audio_disconnect+0x5fb/0x860 sound/usb/card.c:684
usb_unbind_interface+0x216/0xa90 drivers/usb/core/driver.c:423
__device_release_driver drivers/base/dd.c:833 [inline]
device_release_driver_internal+0x4da/0x5a0 drivers/base/dd.c:863
device_release_driver+0x1e/0x30 drivers/base/dd.c:888
bus_remove_device+0x2da/0x490 drivers/base/bus.c:565
device_del+0x5b1/0xaa0 drivers/base/core.c:1853
usb_disable_device+0x1e9/0x680 drivers/usb/core/message.c:1170
usb_disconnect+0x2ca/0x830 drivers/usb/core/hub.c:2124
hub_port_connect drivers/usb/core/hub.c:4741 [inline]
hub_port_connect_change drivers/usb/core/hub.c:4996 [inline]
port_event drivers/usb/core/hub.c:5102 [inline]
hub_event+0x14d8/0x3960 drivers/usb/core/hub.c:5186
process_one_work+0x9fb/0x1570 kernel/workqueue.c:2097
worker_thread+0x1e4/0x1350 kernel/workqueue.c:2231
kthread+0x324/0x3f0 kernel/kthread.c:231
ret_from_fork+0x25/0x30 arch/x86/entry/entry_64.S:425

The buggy address belongs to the object at ffff88006c611980
which belongs to the cache kmalloc-2048 of size 2048
The buggy address is located 1040 bytes inside of
2048-byte region [ffff88006c611980, ffff88006c612180)
The buggy address belongs to the page:
page:ffffea0001b18400 count:1 mapcount:0 mapping: (null)
index:0xffff88006c617700 compound_mapcount: 0
flags: 0x500000000008100(slab|head)
raw: 0500000000008100 0000000000000000 ffff88006c617700 00000001800f0007
raw: ffffea0001b10400 0000000300000003 ffff88003e803040 0000000000000000
page dumped because: kasan: bad access detected

Memory state around the buggy address:
ffff88006c611c80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88006c611d00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
>ffff88006c611d80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
^
ffff88006c611e00: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
ffff88006c611e80: fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb fb
==================================================================

[Takashi Iwai]

So this looks like a pending work triggering the issue when performed
during the card free. What about the patch below as a quick band-aid
fix?


thanks,

Takashi

---
diff --git a/sound/core/seq_device.c b/sound/core/seq_device.c
--- a/sound/core/seq_device.c
+++ b/sound/core/seq_device.c
@@ -148,8 +148,10 @@ void snd_seq_device_load_drivers(void)
flush_work(&autoload_work);
}
EXPORT_SYMBOL(snd_seq_device_load_drivers);
+#define cancel_autoload_drivers() cancel_work_sync(&autoload_work)
#else
#define queue_autoload_drivers() /* NOP */
+#define cancel_autoload_drivers() /* NOP */
#endif

/*
@@ -159,6 +161,7 @@ static int snd_seq_device_dev_free(struct snd_device *device)
{
struct snd_seq_device *dev = device->device_data;

+ cancel_autoload_drivers();
put_device(&dev->dev);
return 0;
}

[Andrey Konovalov]

Hi Takashi,

I've been running syzkaller with your patch for a few days and didn't
see any more of those reports.

Tested-by: Andrey Konovalov <andre...@google.com>

Thanks!

>
> ---
> diff --git a/sound/core/seq_device.c b/sound/core/seq_device.c
> --- a/sound/core/seq_device.c
> +++ b/sound/core/seq_device.c
> @@ -148,8 +148,10 @@ void snd_seq_device_load_drivers(void)
> flush_work(&autoload_work);
> }
> EXPORT_SYMBOL(snd_seq_device_load_drivers);
> +#define cancel_autoload_drivers() cancel_work_sync(&autoload_work)
> #else
> #define queue_autoload_drivers() /* NOP */
> +#define cancel_autoload_drivers() /* NOP */
> #endif
>
> /*
> @@ -159,6 +161,7 @@ static int snd_seq_device_dev_free(struct snd_device *device)
> {
> struct snd_seq_device *dev = device->device_data;
>
> + cancel_autoload_drivers();
> put_device(&dev->dev);
> return 0;
> }
>

> --
> You received this message because you are subscribed to the Google Groups "syzkaller" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to syzkaller+...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[Takashi Iwai]

Thanks, I merged the fix now.
FWIW, below is the official patch I merged.


Takashi

-- 8< --
From: Takashi Iwai <ti...@suse.de>
Subject: [PATCH] ALSA: seq: Cancel pending autoload work at unbinding device

ALSA sequencer core has a mechanism to load the enumerated devices
automatically, and it's performed in an off-load work. This seems
causing some race when a sequencer is removed while the pending
autoload work is running. As syzkaller spotted, it may lead to some
use-after-free:

The fix is simply to assure canceling the autoload work at removing
the device.

Reported-by: Andrey Konovalov <andre...@google.com>
Tested-by: Andrey Konovalov <andre...@google.com>
Cc: <sta...@vger.kernel.org>
Signed-off-by: Takashi Iwai <ti...@suse.de>
---
sound/core/seq_device.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/sound/core/seq_device.c b/sound/core/seq_device.c
index c4acf17e9f5e..e40a2cba5002 100644

--- a/sound/core/seq_device.c
+++ b/sound/core/seq_device.c
@@ -148,8 +148,10 @@ void snd_seq_device_load_drivers(void)
flush_work(&autoload_work);
}
EXPORT_SYMBOL(snd_seq_device_load_drivers);
+#define cancel_autoload_drivers() cancel_work_sync(&autoload_work)
#else
#define queue_autoload_drivers() /* NOP */
+#define cancel_autoload_drivers() /* NOP */
#endif

/*
@@ -159,6 +161,7 @@ static int snd_seq_device_dev_free(struct snd_device *device)
{
struct snd_seq_device *dev = device->device_data;

+ cancel_autoload_drivers();
put_device(&dev->dev);
return 0;
}
--

2.14.1

[Andrey Konovalov]

Great, thanks!