kernel.git/sound/core/rawmidi.c, branch linux-3.2.y

ALSA: rawmidi: Avoid racy info ioctl via ctl device

2018-03-03T15:50:47Z

commit c1cfd9025cc394fd137a01159d74335c5ac978ce upstream. The rawmidi also allows to obtaining the information via ioctl of ctl API. It means that user can issue an ioctl to the rawmidi device even when it's being removed as long as the control device is present. Although the code has some protection via the global register_mutex, its range is limited to the search of the corresponding rawmidi object, and the mutex is already unlocked at accessing the rawmidi object. This may lead to a use-after-free. For avoiding it, this patch widens the application of register_mutex to the whole snd_rawmidi_info_select() function. We have another mutex per rawmidi object, but this operation isn't very hot path, so it shouldn't matter from the performance POV. Signed-off-by: Takashi Iwai [bwh: Backported to 3.2: adjust context] Signed-off-by: Ben Hutchings

ALSA: rawmidi: Fix possible deadlock with virmidi registration

2016-11-20T01:01:37Z

commit 816f318b2364262a51024096da7ca3b84e78e3b5 upstream. When a seq-virmidi driver is initialized, it registers a rawmidi instance with its callback to create an associated seq kernel client. Currently it's done throughly in rawmidi's register_mutex context. Recently it was found that this may lead to a deadlock another rawmidi device that is being attached with the sequencer is accessed, as both open with the same register_mutex. This was actually triggered by syzkaller, as Dmitry Vyukov reported: ====================================================== [ INFO: possible circular locking dependency detected ] 4.8.0-rc1+ #11 Not tainted ------------------------------------------------------- syz-executor/7154 is trying to acquire lock: (register_mutex#5){+.+.+.}, at: [] snd_rawmidi_kernel_open+0x4b/0x260 sound/core/rawmidi.c:341 but task is already holding lock: (&grp->list_mutex){++++.+}, at: [] check_and_subscribe_port+0x5b/0x5c0 sound/core/seq/seq_ports.c:495 which lock already depends on the new lock. the existing dependency chain (in reverse order) is: -> #1 (&grp->list_mutex){++++.+}: [] lock_acquire+0x208/0x430 kernel/locking/lockdep.c:3746 [] down_read+0x49/0xc0 kernel/locking/rwsem.c:22 [< inline >] deliver_to_subscribers sound/core/seq/seq_clientmgr.c:681 [] snd_seq_deliver_event+0x35e/0x890 sound/core/seq/seq_clientmgr.c:822 [] > snd_seq_kernel_client_dispatch+0x126/0x170 sound/core/seq/seq_clientmgr.c:2418 [] snd_seq_system_broadcast+0xb2/0xf0 sound/core/seq/seq_system.c:101 [] snd_seq_create_kernel_client+0x24a/0x330 sound/core/seq/seq_clientmgr.c:2297 [< inline >] snd_virmidi_dev_attach_seq sound/core/seq/seq_virmidi.c:383 [] snd_virmidi_dev_register+0x29f/0x750 sound/core/seq/seq_virmidi.c:450 [] snd_rawmidi_dev_register+0x30c/0xd40 sound/core/rawmidi.c:1645 [] __snd_device_register.part.0+0x63/0xc0 sound/core/device.c:164 [< inline >] __snd_device_register sound/core/device.c:162 [] snd_device_register_all+0xad/0x110 sound/core/device.c:212 [] snd_card_register+0xef/0x6c0 sound/core/init.c:749 [] snd_virmidi_probe+0x3ef/0x590 sound/drivers/virmidi.c:123 [] platform_drv_probe+0x8b/0x170 drivers/base/platform.c:564 ...... -> #0 (register_mutex#5){+.+.+.}: [< inline >] check_prev_add kernel/locking/lockdep.c:1829 [< inline >] check_prevs_add kernel/locking/lockdep.c:1939 [< inline >] validate_chain kernel/locking/lockdep.c:2266 [] __lock_acquire+0x4d44/0x4d80 kernel/locking/lockdep.c:3335 [] lock_acquire+0x208/0x430 kernel/locking/lockdep.c:3746 [< inline >] __mutex_lock_common kernel/locking/mutex.c:521 [] mutex_lock_nested+0xb1/0xa20 kernel/locking/mutex.c:621 [] snd_rawmidi_kernel_open+0x4b/0x260 sound/core/rawmidi.c:341 [] midisynth_subscribe+0xf7/0x350 sound/core/seq/seq_midi.c:188 [< inline >] subscribe_port sound/core/seq/seq_ports.c:427 [] check_and_subscribe_port+0x467/0x5c0 sound/core/seq/seq_ports.c:510 [] snd_seq_port_connect+0x2c9/0x500 sound/core/seq/seq_ports.c:579 [] snd_seq_ioctl_subscribe_port+0x1d8/0x2b0 sound/core/seq/seq_clientmgr.c:1480 [] snd_seq_do_ioctl+0x184/0x1e0 sound/core/seq/seq_clientmgr.c:2225 [] snd_seq_kernel_client_ctl+0xa8/0x110 sound/core/seq/seq_clientmgr.c:2440 [] snd_seq_oss_midi_open+0x3b4/0x610 sound/core/seq/oss/seq_oss_midi.c:375 [] snd_seq_oss_synth_setup_midi+0x107/0x4c0 sound/core/seq/oss/seq_oss_synth.c:281 [] snd_seq_oss_open+0x748/0x8d0 sound/core/seq/oss/seq_oss_init.c:274 [] odev_open+0x6a/0x90 sound/core/seq/oss/seq_oss.c:138 [] soundcore_open+0x30f/0x640 sound/sound_core.c:639 ...... other info that might help us debug this: Possible unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&grp->list_mutex); lock(register_mutex#5); lock(&grp->list_mutex); lock(register_mutex#5); *** DEADLOCK *** ====================================================== The fix is to simply move the registration parts in snd_rawmidi_dev_register() to the outside of the register_mutex lock. The lock is needed only to manage the linked list, and it's not necessarily to cover the whole initialization process. Reported-by: Dmitry Vyukov Signed-off-by: Takashi Iwai [bwh: Backported to 3.2: adjust context] Signed-off-by: Ben Hutchings

ALSA: rawmidi: Fix race at copying & updating the position

2016-02-27T14:28:46Z

commit 81f577542af15640cbcb6ef68baa4caa610cbbfc upstream. The rawmidi read and write functions manage runtime stream status such as runtime->appl_ptr and runtime->avail. These point where to copy the new data and how many bytes have been copied (or to be read). The problem is that rawmidi read/write call copy_from_user() or copy_to_user(), and the runtime spinlock is temporarily unlocked and relocked while copying user-space. Since the current code advances and updates the runtime status after the spin unlock/relock, the copy and the update may be asynchronous, and eventually runtime->avail might go to a negative value when many concurrent accesses are done. This may lead to memory corruption in the end. For fixing this race, in this patch, the status update code is performed in the same lock before the temporary unlock. Also, the spinlock is now taken more widely in snd_rawmidi_kernel_read1() for protecting more properly during the whole operation. BugLink: http://lkml.kernel.org/r/CACT4Y+b-dCmNf1GpgPKfDO0ih+uZCL2JV4__j-r1kdhPLSgQCQ@mail.gmail.com Reported-by: Dmitry Vyukov Tested-by: Dmitry Vyukov Signed-off-by: Takashi Iwai Signed-off-by: Ben Hutchings

ALSA: rawmidi: Make snd_rawmidi_transmit() race-free

2016-02-27T14:28:46Z

commit 06ab30034ed9c200a570ab13c017bde248ddb2a6 upstream. A kernel WARNING in snd_rawmidi_transmit_ack() is triggered by syzkaller fuzzer: WARNING: CPU: 1 PID: 20739 at sound/core/rawmidi.c:1136 Call Trace: [< inline >] __dump_stack lib/dump_stack.c:15 [] dump_stack+0x6f/0xa2 lib/dump_stack.c:50 [] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482 [] warn_slowpath_null+0x29/0x30 kernel/panic.c:515 [] snd_rawmidi_transmit_ack+0x275/0x400 sound/core/rawmidi.c:1136 [] snd_virmidi_output_trigger+0x4b1/0x5a0 sound/core/seq/seq_virmidi.c:163 [< inline >] snd_rawmidi_output_trigger sound/core/rawmidi.c:150 [] snd_rawmidi_kernel_write1+0x549/0x780 sound/core/rawmidi.c:1223 [] snd_rawmidi_write+0x543/0xb30 sound/core/rawmidi.c:1273 [] __vfs_write+0x113/0x480 fs/read_write.c:528 [] vfs_write+0x167/0x4a0 fs/read_write.c:577 [< inline >] SYSC_write fs/read_write.c:624 [] SyS_write+0x111/0x220 fs/read_write.c:616 [] entry_SYSCALL_64_fastpath+0x16/0x7a arch/x86/entry/entry_64.S:185 Also a similar warning is found but in another path: Call Trace: [< inline >] __dump_stack lib/dump_stack.c:15 [] dump_stack+0x6f/0xa2 lib/dump_stack.c:50 [] warn_slowpath_common+0xd9/0x140 kernel/panic.c:482 [] warn_slowpath_null+0x29/0x30 kernel/panic.c:515 [] rawmidi_transmit_ack+0x24a/0x3b0 sound/core/rawmidi.c:1133 [] snd_rawmidi_transmit_ack+0x51/0x80 sound/core/rawmidi.c:1163 [] snd_virmidi_output_trigger+0x2b6/0x570 sound/core/seq/seq_virmidi.c:185 [< inline >] snd_rawmidi_output_trigger sound/core/rawmidi.c:150 [] snd_rawmidi_kernel_write1+0x4bb/0x760 sound/core/rawmidi.c:1252 [] snd_rawmidi_write+0x543/0xb30 sound/core/rawmidi.c:1302 [] __vfs_write+0x113/0x480 fs/read_write.c:528 [] vfs_write+0x167/0x4a0 fs/read_write.c:577 [< inline >] SYSC_write fs/read_write.c:624 [] SyS_write+0x111/0x220 fs/read_write.c:616 [] entry_SYSCALL_64_fastpath+0x16/0x7a arch/x86/entry/entry_64.S:185 In the former case, the reason is that virmidi has an open code calling snd_rawmidi_transmit_ack() with the value calculated outside the spinlock. We may use snd_rawmidi_transmit() in a loop just for consuming the input data, but even there, there is a race between snd_rawmidi_transmit_peek() and snd_rawmidi_tranmit_ack(). Similarly in the latter case, it calls snd_rawmidi_transmit_peek() and snd_rawmidi_tranmit_ack() separately without protection, so they are racy as well. The patch tries to address these issues by the following ways: - Introduce the unlocked versions of snd_rawmidi_transmit_peek() and snd_rawmidi_transmit_ack() to be called inside the explicit lock. - Rewrite snd_rawmidi_transmit() to be race-free (the former case). - Make the split calls (the latter case) protected in the rawmidi spin lock. BugLink: http://lkml.kernel.org/r/CACT4Y+YPq1+cYLkadwjWa5XjzF1_Vki1eHnVn-Lm0hzhSpu5PA@mail.gmail.com BugLink: http://lkml.kernel.org/r/CACT4Y+acG4iyphdOZx47Nyq_VHGbpJQK-6xNpiqUjaZYqsXOGw@mail.gmail.com Reported-by: Dmitry Vyukov Tested-by: Dmitry Vyukov Signed-off-by: Takashi Iwai [bwh: Backported to 3.2: adjust context] Signed-off-by: Ben Hutchings

ALSA: rawmidi: Remove kernel WARNING for NULL user-space buffer check

2016-02-27T14:28:45Z

commit cc85f7a634cfaf9f0713c6aa06d08817424db37a upstream. NULL user-space buffer can be passed even in a normal path, thus it's not good to spew a kernel warning with stack trace at each time. Just drop snd_BUG_ON() macro usage there. BugLink: http://lkml.kernel.org/r/CACT4Y+YfVJ3L+q0i-4vyQVyyPD7V=OMX0PWPi29x9Bo3QaBLdw@mail.gmail.com Reported-by: Dmitry Vyukov Signed-off-by: Takashi Iwai Signed-off-by: Ben Hutchings

ALSA: Avoid endless sleep after disconnect

2012-11-16T16:46:50Z

commit 0914f7961babbf28aaa2f19b453951fb4841c03f upstream. When disconnect callback is called, each component should wake up sleepers and check card->shutdown flag for avoiding the endless sleep blocking the proper resource release. Signed-off-by: Takashi Iwai Signed-off-by: Ben Hutchings

ALSA: Add a reference counter to card instance

2012-11-16T16:46:50Z

commit a0830dbd4e42b38aefdf3fb61ba5019a1a99ea85 upstream. For more strict protection for wild disconnections, a refcount is introduced to the card instance, and let it up/down when an object is referred via snd_lookup_*() in the open ops. The free-after-last-close check is also changed to check this refcount instead of the empty list, too. Reported-by: Matthieu CASTET Signed-off-by: Takashi Iwai Signed-off-by: Ben Hutchings

sound: fix drivers needing module.h not moduleparam.h

2011-10-31T23:31:19Z

The implicit presence of module.h lured several users into incorrectly thinking that they only needed/used modparam.h but once we clean up the module.h presence, these will show up as build failures, so fix 'em now. Signed-off-by: Paul Gortmaker

ALSA: Use krealloc() in possible places

2011-07-12T06:05:16Z

Signed-off-by: Takashi Iwai

ALSA: rawmidi - Use workq for event handling

2011-06-14T12:37:06Z

Kill tasklet usage in rawmidi core code. Use workq for the event callback instead of tasklet (which is used only in core/seq/seq_midi.c). Signed-off-by: Takashi Iwai