[alsa-devel] pcm_lock deadlock

[Ville Syrjälä]

On Tue, Oct 29, 2019 at 09:52:57PM +0100, Takashi Iwai wrote:
> On Tue, 29 Oct 2019 20:10:50 +0100,
> From: Takashi Iwai <tiwai at suse.de>
> Subject: [PATCH] ALSA: hda - Fix mutex deadlock in HDMI codec driver
> 
> The commit ade49db337a9 ("ALSA: hda/hdmi - Allow audio component for
> AMD/ATI and Nvidia HDMI") introduced the spec->pcm_lock mutex lock to
> the whole generic_hdmi_init() function for avoiding the race with the
> audio component registration.  However, this caused a dead lock when
> the unsolicited event is handled without the audio component, as the
> codec gets runtime-resumed in hdmi_present_sense() which is already
> inside the spec->pcm_lock in its caller.
> 
> For avoiding this deadlock, add a new mutex only for the audio
> component binding that is used in both generic_hdmi_init() and the
> audio notifier registration where the jack callbacks are handled /
> re-registered.
> 
> Fixes: ade49db337a9 ("ALSA: hda/hdmi - Allow audio component for AMD/ATI and Nvidia HDMI")
> Reported-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Signed-off-by: Takashi Iwai <tiwai at suse.de>
> ---
>  sound/pci/hda/patch_hdmi.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 795cbda32cbb..d9b5ba361409 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -145,6 +145,7 @@ struct hdmi_spec {
>  	struct snd_array pins; /* struct hdmi_spec_per_pin */
>  	struct hdmi_pcm pcm_rec[16];
>  	struct mutex pcm_lock;
> +	struct mutex bind_lock; /* for audio component binding */

Missing mutex_init() for this guy.

Tested-by: Ville Syrjälä <ville.syrjala at linux.intel.com>

>  	/* pcm_bitmap means which pcms have been assigned to pins*/
>  	unsigned long pcm_bitmap;
>  	int pcm_used;	/* counter of pcm_rec[] */
> @@ -2258,7 +2259,7 @@ static int generic_hdmi_init(struct hda_codec *codec)
>  	struct hdmi_spec *spec = codec->spec;
>  	int pin_idx;
>  
> -	mutex_lock(&spec->pcm_lock);
> +	mutex_lock(&spec->bind_lock);
>  	spec->use_jack_detect = !codec->jackpoll_interval;
>  	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
>  		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
> @@ -2275,7 +2276,7 @@ static int generic_hdmi_init(struct hda_codec *codec)
>  			snd_hda_jack_detect_enable_callback(codec, pin_nid,
>  							    jack_callback);
>  	}
> -	mutex_unlock(&spec->pcm_lock);
> +	mutex_unlock(&spec->bind_lock);
>  	return 0;
>  }
>  
> @@ -2451,7 +2452,7 @@ static void generic_acomp_notifier_set(struct drm_audio_component *acomp,
>  	int i;
>  
>  	spec = container_of(acomp->audio_ops, struct hdmi_spec, drm_audio_ops);
> -	mutex_lock(&spec->pcm_lock);
> +	mutex_lock(&spec->bind_lock);
>  	spec->use_acomp_notifier = use_acomp;
>  	spec->codec->relaxed_resume = use_acomp;
>  	/* reprogram each jack detection logic depending on the notifier */
> @@ -2461,7 +2462,7 @@ static void generic_acomp_notifier_set(struct drm_audio_component *acomp,
>  					      get_pin(spec, i)->pin_nid,
>  					      use_acomp);
>  	}
> -	mutex_unlock(&spec->pcm_lock);
> +	mutex_unlock(&spec->bind_lock);
>  }
>  
>  /* enable / disable the notifier via master bind / unbind */
> -- 
> 2.16.4

-- 
Ville Syrjälä
Intel