Commit 96e503f9 authored by Takashi Iwai's avatar Takashi Iwai
Browse files

ALSA: hda/i915 - fix list corruption with concurrent probes 



Current hdac_i915 uses a static completion instance to wait
for i915 driver to complete the component bind.

This design is not safe if multiple HDA controllers are active and
communicating with different i915 instances, and can lead to list
corruption and failed audio driver probe.

Fix the design by moving completion mechanism to common acomp
code and remove the related code from hdac_i915.

Fixes: 7b882fe3 ("ALSA: hda - handle multiple i915 device instances")
Co-developed-by: default avatarKai Vehmanen <kai.vehmanen@linux.intel.com>
Signed-off-by: default avatarKai Vehmanen <kai.vehmanen@linux.intel.com>
Link: https://lore.kernel.org/r/20201006161722.500256-1-kai.vehmanen@linux.intel.com


Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
parent 7dcd5612

include/drm/drm_audio_component.h

+4 −0
Original line number Diff line number Diff line
@@ -117,6 +117,10 @@ struct drm_audio_component { 
	 * @audio_ops: Ops implemented by hda driver, called by DRM driver

	 */

	const struct drm_audio_component_audio_ops *audio_ops;

	/**

	 * @master_bind_complete: completion held during component master binding

	 */

	struct completion master_bind_complete;

};



#endif /* _DRM_AUDIO_COMPONENT_H_ */

sound/hda/hdac_component.c

+3 −0
Original line number Diff line number Diff line
@@ -210,12 +210,14 @@ static int hdac_component_master_bind(struct device *dev) 
			goto module_put;

	}



	complete_all(&acomp->master_bind_complete);

	return 0;



 module_put:

	module_put(acomp->ops->owner);

out_unbind:

	component_unbind_all(dev, acomp);

	complete_all(&acomp->master_bind_complete);



	return ret;

}
@@ -296,6 +298,7 @@ int snd_hdac_acomp_init(struct hdac_bus *bus, 
	if (!acomp)

		return -ENOMEM;

	acomp->audio_ops = aops;

	init_completion(&acomp->master_bind_complete);

	bus->audio_component = acomp;

	devres_add(dev, acomp);

sound/hda/hdac_i915.c

+3 −20
Original line number Diff line number Diff line
@@ -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) || \
@@ -130,19 +128,6 @@ static bool i915_gfx_present(void) 
	return pci_dev_present(ids);

}



static int i915_master_bind(struct device *dev,

			    struct drm_audio_component *acomp)

{

	complete_all(&bind_complete);

	/* clear audio_ops here as it was needed only for completion call */

	acomp->audio_ops = NULL;

	return 0;

}



static const struct drm_audio_component_audio_ops i915_init_ops = {

	.master_bind = i915_master_bind

};



/**

 * snd_hdac_i915_init - Initialize i915 audio component

 * @bus: HDA core bus
@@ -163,9 +148,7 @@ int snd_hdac_i915_init(struct hdac_bus *bus) 
	if (!i915_gfx_present())

		return -ENODEV;



	init_completion(&bind_complete);



	err = snd_hdac_acomp_init(bus, &i915_init_ops,

	err = snd_hdac_acomp_init(bus, NULL,

				  i915_component_master_match,

				  sizeof(struct i915_audio_component) - sizeof(*acomp));

	if (err < 0)
@@ -177,7 +160,7 @@ 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,

			wait_for_completion_timeout(&acomp->master_bind_complete,

						    msecs_to_jiffies(60 * 1000));

		}

	}

CRA Git | Maintained and supported by SUSTech CRA and CCSE