From e2dff1bd3b529b8ebcc4142f1a9041b819561340 Mon Sep 17 00:00:00 2001 From: Yan Yankovskyi Date: Fri, 18 Jun 2021 14:07:40 +0200 Subject: [PATCH] mse: core: Fix potential deadlock during audio transmission start refs #326355 mse_work_start_transmission_common() and mse_start_transmission() deal with, among others, two spin locks: lock_buf_list and lock_state. It's possible that these two locks will be acquired in opposite orders in two parallel threads, potentially resulting in a deadlock. Do not try to acquire lock_state lock while holding lock_buf_list to avoid this situation. Example command to trigger the warning: $ speaker-test -r 48000 -t sine -c 2 -F S16_BE -p 16000 -Dhw:ravbmse,1 Reproduction rate: 100%. LOCKDEP warning: [ 34.776517] mse_packetq/2167 is trying to acquire lock: [ 34.783138] (&instance->lock_state){....}, at: [] mse_work_start_transmission_common+0xc8/0x428 [mse_core] [ 34.795864] [ 34.795864] but task is already holding lock: [ 34.801957] (&(&instance->lock_buf_list)->rlock){....}, at: [] mse_work_start_transmission_common+0x60/0x428 [mse_core] [ 34.814572] [ 34.814572] which lock already depends on the new lock. [ 34.814572] [ 34.822833] [ 34.822833] the existing dependency chain (in reverse order) is: [ 34.830396] [ 34.830396] -> #1 (&(&instance->lock_buf_list)->rlock){....}: [ 34.837722] _raw_spin_lock_irqsave+0x58/0x98 [ 34.842732] mse_start_transmission+0x178/0x2e8 [mse_core] [ 34.848826] mse_adapter_alsa_trigger+0x100/0x1cc [mse_adapter_alsa] [ 34.855788] snd_pcm_do_start+0x28/0x38 [ 34.860211] snd_pcm_action_single+0x44/0x7c [ 34.865071] snd_pcm_action+0x24/0xf8 [ 34.869324] snd_pcm_start+0x20/0x28 [ 34.873488] __snd_pcm_lib_xfer+0x588/0x624 [ 34.878261] snd_pcm_common_ioctl+0x484/0x1024 [ 34.883296] snd_pcm_ioctl+0x28/0x3c [ 34.887465] vfs_ioctl+0x24/0x40 [ 34.891278] do_vfs_ioctl+0xa4/0x890 [ 34.895439] SyS_ioctl+0x5c/0xd0 [ 34.899257] el0_svc_naked+0x34/0x38 [ 34.903416] [ 34.903416] -> #0 (&instance->lock_state){....}: [ 34.909593] lock_acquire+0x22c/0x25c [ 34.913842] _raw_read_lock_irqsave+0x58/0x98 [ 34.918871] mse_work_start_transmission_common+0xc8/0x428 [mse_core] [ 34.925983] mse_work_start_transmission+0x14c/0x150 [mse_core] [ 34.932508] kthread_worker_fn+0x110/0x188 [ 34.937194] kthread+0x11c/0x12c [ 34.941010] ret_from_fork+0x10/0x18 [ 34.945170] [ 34.945170] other info that might help us debug this: [ 34.945170] [ 34.953256] Possible unsafe locking scenario: [ 34.953256] [ 34.959249] CPU0 CPU1 [ 34.963844] ---- ---- [ 34.968438] lock(&(&instance->lock_buf_list)->rlock); [ 34.973733] lock(&instance->lock_state); [ 34.980422] lock(&(&instance->lock_buf_list)->rlock); [ 34.988248] lock(&instance->lock_state); [ 34.992408] [ 34.992408] *** DEADLOCK *** Signed-off-by: Yan Yankovskyi Signed-off-by: Masaru Nagai --- mse_core_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mse_core_main.c b/mse_core_main.c index a863251..9e8acc2 100644 --- a/mse_core_main.c +++ b/mse_core_main.c @@ -4207,7 +4207,7 @@ static void mse_work_start_transmission_common(struct mse_instance *instance) return; } else if (IS_MSE_TYPE_AUDIO(instance->media->type)) { - if (mse_state_test(instance, MSE_STATE_STOPPING)) { + if (mse_state_test_nolock(instance, MSE_STATE_STOPPING)) { spin_unlock_irqrestore(&instance->lock_buf_list, flags); queue_work(instance->wq_packet,