diff mbox series

[RFC,2/2] ALSA: hda/i915 - fix list corruption with concurrent probes

Message ID 20201006113042.471718-3-kai.vehmanen@linux.intel.com
State New
Headers show
Series ALSA: hda - acomp probe fix for i915 | expand

Commit Message

Kai Vehmanen Oct. 6, 2020, 11:30 a.m. UTC
Current hdac_i915 uses a static completion instance to synchronous
communication with i915 driver. This design is not safe if multiple
HDA controllers are active and talking to different i915 instances,
and can lead to list corruption and failed audio driver probe.

Fix the design by storing the completion object to hdac_bus,
and signaling completions on a per-bus basis.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 include/sound/hdaudio.h |  2 ++
 sound/hda/hdac_i915.c   | 16 +++++++++-------
 2 files changed, 11 insertions(+), 7 deletions(-)
diff mbox series

Patch

diff --git a/include/sound/hdaudio.h b/include/sound/hdaudio.h
index 6eed61e6cf8a..116c89074d79 100644
--- a/include/sound/hdaudio.h
+++ b/include/sound/hdaudio.h
@@ -6,6 +6,7 @@ 
 #ifndef __SOUND_HDAUDIO_H
 #define __SOUND_HDAUDIO_H
 
+#include <linux/completion.h>
 #include <linux/device.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
@@ -359,6 +360,7 @@  struct hdac_bus {
 	struct drm_audio_component *audio_component;
 	long display_power_status;
 	unsigned long display_power_active;
+	struct completion display_bind_complete;
 
 	/* parameters required for enhanced capabilities */
 	int num_streams;
diff --git a/sound/hda/hdac_i915.c b/sound/hda/hdac_i915.c
index 5f0a1aa6ad84..fee3d379d7e0 100644
--- a/sound/hda/hdac_i915.c
+++ b/sound/hda/hdac_i915.c
@@ -11,8 +11,6 @@ 
 #include <sound/hda_i915.h>
 #include <sound/hda_register.h>
 
-static struct completion bind_complete;
-
 #define IS_HSW_CONTROLLER(pci) (((pci)->device == 0x0a0c) || \
 				((pci)->device == 0x0c0c) || \
 				((pci)->device == 0x0d0c) || \
@@ -133,9 +131,14 @@  static bool i915_gfx_present(void)
 static int i915_master_bind(struct device *dev,
 			    struct drm_audio_component *acomp)
 {
-	complete_all(&bind_complete);
+	struct hdac_bus *bus = snd_hdac_acomp_get_bus(dev);
+
+	if (!bus)
+		return -EINVAL;
+	complete_all(&bus->display_bind_complete);
 	/* clear audio_ops here as it was needed only for completion call */
 	acomp->audio_ops = NULL;
+
 	return 0;
 }
 
@@ -163,8 +166,7 @@  int snd_hdac_i915_init(struct hdac_bus *bus)
 	if (!i915_gfx_present())
 		return -ENODEV;
 
-	init_completion(&bind_complete);
-
+	init_completion(&bus->display_bind_complete);
 	err = snd_hdac_acomp_init(bus, &i915_init_ops,
 				  i915_component_master_match,
 				  sizeof(struct i915_audio_component) - sizeof(*acomp));
@@ -177,8 +179,8 @@  int snd_hdac_i915_init(struct hdac_bus *bus)
 		if (!IS_ENABLED(CONFIG_MODULES) ||
 		    !request_module("i915")) {
 			/* 60s timeout */
-			wait_for_completion_timeout(&bind_complete,
-						   msecs_to_jiffies(60 * 1000));
+			wait_for_completion_timeout(&bus->display_bind_complete,
+						    msecs_to_jiffies(60 * 1000));
 		}
 	}
 	if (!acomp->ops) {