Skip to contentSkip to conversation

Patch "ALSA: hda - Fix deadlock of controller device lock at unbinding" has been added to the 4.4-stable tree

PATCH [email protected] 1 participant 1 message May 11, 2017

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

ALSA: hda - Fix deadlock of controller device lock at unbinding

to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
alsa-hda-fix-deadlock-of-controller-device-lock-at-unbinding.patch
and it can be found in the queue-4.4 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From ab949d519601880fd46e8bc1445d6a453bf2dc09 Mon Sep 17 00:00:00 2001

From: Takashi Iwai <[email protected]>
Date: Mon, 2 Jan 2017 11:37:04 +0100
Subject: ALSA: hda - Fix deadlock of controller device lock at unbinding

From: Takashi Iwai <[email protected]>

commit ab949d519601880fd46e8bc1445d6a453bf2dc09 upstream.

Imre Deak reported a deadlock of HD-audio driver at unbinding while it's still in probing. Since we probe the codecs asynchronously in a work, the codec driver probe may still be kicked off while the controller itself is being unbound. And, azx_remove() tries to process all pending tasks via cancel_work_sync() for fixing the other races (see commit [0b8c82190c12: ALSA: hda - Cancel probe work instead of flush at remove]), now we may meet a bizarre deadlock:

Unbind snd_hda_intel via sysfs:
device_release_driver() ->

    device_lock(snd_hda_intel) ->
      azx_remove() ->
        cancel_work_sync(azx_probe_work)

azx_probe_work():
codec driver probe() ->

     __driver_attach() ->
       device_lock(snd_hda_intel)

This deadlock is caused by the fact that both device_release_driver() and driver_probe_device() take both the device and its parent locks at the same time. The codec device sets the controller device as its parent, and this lock is taken before the probe() callback is called, while the controller remove() callback gets called also with the same lock.

In this patch, as an ugly workaround, we unlock the controller device temporarily during cancel_work_sync() call. The race against another bind call should be still suppressed by the parent's device lock.

Fixes 0b8c82190c12 ("ALSA: hda - Cancel probe work instead of flush at remove")
---
 sound/pci/hda/hda_intel.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)
sound/pci/hda/hda_intel.c +13 -0
@@ -2166,7 +2166,20 @@ static void azx_remove(struct pci_dev *p
 		/* cancel the pending probing work */
 		chip = card->private_data;
 		hda = container_of(chip, struct hda_intel, chip);
+		/* FIXME: below is an ugly workaround.
+		 * Both device_release_driver() and driver_probe_device()
+		 * take *both* the device's and its parent's lock before
+		 * calling the remove() and probe() callbacks.  The codec
+		 * probe takes the locks of both the codec itself and its
+		 * parent, i.e. the PCI controller dev.  Meanwhile, when
+		 * the PCI controller is unbound, it takes its lock, too
+		 * ==> ouch, a deadlock!
+		 * As a workaround, we unlock temporarily here the controller
+		 * device during cancel_work_sync() call.
+		 */
+		device_unlock(&pci->dev);
 		cancel_work_sync(&hda->probe_work);
+		device_lock(&pci->dev);
 
 		snd_card_free(card);
 	}

Patches currently in stable-queue which might be from [email protected] are

queue-4.4/alsa-hda-fix-deadlock-of-controller-device-lock-at-unbinding.patch