Skip to content

Commit c7a6065

Browse files
committed
ALSA: info: Fix potential deadlock at disconnection
As reported recently, ALSA core info helper may cause a deadlock at the forced device disconnection during the procfs operation. The proc_remove() (that is called from the snd_card_disconnect() helper) has a synchronization of the pending procfs accesses via wait_for_completion(). Meanwhile, ALSA procfs helper takes the global mutex_lock(&info_mutex) at both the proc_open callback and snd_card_info_disconnect() helper. Since the proc_open can't finish due to the mutex lock, wait_for_completion() never returns, either, hence it deadlocks. TASK#1 TASK#2 proc_reg_open() takes use_pde() snd_info_text_entry_open() snd_card_disconnect() snd_info_card_disconnect() takes mutex_lock(&info_mutex) proc_remove() wait_for_completion(unused_pde) ... waiting task#1 closes mutex_lock(&info_mutex) => DEADLOCK This patch is a workaround for avoiding the deadlock scenario above. The basic strategy is to move proc_remove() call outside the mutex lock. proc_remove() can work gracefully without extra locking, and it can delete the tree recursively alone. So, we call proc_remove() at snd_info_card_disconnection() at first, then delete the rest resources recursively within the info_mutex lock. After the change, the function snd_info_disconnect() doesn't do disconnection by itself any longer, but it merely clears the procfs pointer. So rename the function to snd_info_clear_entries() for avoiding confusion. The similar change is applied to snd_info_free_entry(), too. Since the proc_remove() is called only conditionally with the non-NULL entry->p, it's skipped after the snd_info_clear_entries() call. Reported-by: Shinhyung Kang <s47.kang@samsung.com> Closes: https://lore.kernel.org/r/664457955.21699345385931.JavaMail.epsvc@epcpadp4 Reviewed-by: Jaroslav Kysela <perex@perex.cz> Cc: <stable@vger.kernel.org> Link: https://lore.kernel.org/r/20231109141954.4283-1-tiwai@suse.de Signed-off-by: Takashi Iwai <tiwai@suse.de>
1 parent 3e3ab46 commit c7a6065

File tree

1 file changed

+13
-8
lines changed

1 file changed

+13
-8
lines changed

sound/core/info.c

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ struct snd_info_private_data {
5656
};
5757

5858
static int snd_info_version_init(void);
59-
static void snd_info_disconnect(struct snd_info_entry *entry);
59+
static void snd_info_clear_entries(struct snd_info_entry *entry);
6060

6161
/*
6262
@@ -569,11 +569,16 @@ void snd_info_card_disconnect(struct snd_card *card)
569569
{
570570
if (!card)
571571
return;
572-
mutex_lock(&info_mutex);
572+
573573
proc_remove(card->proc_root_link);
574-
card->proc_root_link = NULL;
575574
if (card->proc_root)
576-
snd_info_disconnect(card->proc_root);
575+
proc_remove(card->proc_root->p);
576+
577+
mutex_lock(&info_mutex);
578+
if (card->proc_root)
579+
snd_info_clear_entries(card->proc_root);
580+
card->proc_root_link = NULL;
581+
card->proc_root = NULL;
577582
mutex_unlock(&info_mutex);
578583
}
579584

@@ -745,15 +750,14 @@ struct snd_info_entry *snd_info_create_card_entry(struct snd_card *card,
745750
}
746751
EXPORT_SYMBOL(snd_info_create_card_entry);
747752

748-
static void snd_info_disconnect(struct snd_info_entry *entry)
753+
static void snd_info_clear_entries(struct snd_info_entry *entry)
749754
{
750755
struct snd_info_entry *p;
751756

752757
if (!entry->p)
753758
return;
754759
list_for_each_entry(p, &entry->children, list)
755-
snd_info_disconnect(p);
756-
proc_remove(entry->p);
760+
snd_info_clear_entries(p);
757761
entry->p = NULL;
758762
}
759763

@@ -770,8 +774,9 @@ void snd_info_free_entry(struct snd_info_entry * entry)
770774
if (!entry)
771775
return;
772776
if (entry->p) {
777+
proc_remove(entry->p);
773778
mutex_lock(&info_mutex);
774-
snd_info_disconnect(entry);
779+
snd_info_clear_entries(entry);
775780
mutex_unlock(&info_mutex);
776781
}
777782

0 commit comments

Comments
 (0)