Commit 4ef45229 authored Nov 09, 2023 by Takashi Iwai Committed by Greg Kroah-Hartman Nov 28, 2023

ALSA: info: Fix potential deadlock at disconnection



commit c7a60651953359f98dbf24b43e1bf561e1573ed4 upstream.

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>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

parent c7df9523

sound/core/info.c

+13 −8

Original line number	Diff line number	Diff line
		@@ -57,7 +57,7 @@ struct snd_info_private_data {
		};

		static int snd_info_version_init(void);
		static void snd_info_disconnect(struct snd_info_entry *entry);
		static void snd_info_clear_entries(struct snd_info_entry *entry);

		/*

		@@ -572,11 +572,16 @@ void snd_info_card_disconnect(struct snd_card *card)
		{
		if (!card)
		return;
		mutex_lock(&info_mutex);

		proc_remove(card->proc_root_link);
		card->proc_root_link = NULL;
		if (card->proc_root)
		snd_info_disconnect(card->proc_root);
		proc_remove(card->proc_root->p);

		mutex_lock(&info_mutex);
		if (card->proc_root)
		snd_info_clear_entries(card->proc_root);
		card->proc_root_link = NULL;
		card->proc_root = NULL;
		mutex_unlock(&info_mutex);
		}

		@@ -748,15 +753,14 @@ struct snd_info_entry snd_info_create_card_entry(struct snd_card card,
		}
		EXPORT_SYMBOL(snd_info_create_card_entry);

		static void snd_info_disconnect(struct snd_info_entry *entry)
		static void snd_info_clear_entries(struct snd_info_entry *entry)
		{
		struct snd_info_entry *p;

		if (!entry->p)
		return;
		list_for_each_entry(p, &entry->children, list)
		snd_info_disconnect(p);
		proc_remove(entry->p);
		snd_info_clear_entries(p);
		entry->p = NULL;
		}

		@@ -773,8 +777,9 @@ void snd_info_free_entry(struct snd_info_entry * entry)
		if (!entry)
		return;
		if (entry->p) {
		proc_remove(entry->p);
		mutex_lock(&info_mutex);
		snd_info_disconnect(entry);
		snd_info_clear_entries(entry);
		mutex_unlock(&info_mutex);
		}