[PATCH] ALSA: pcm: Protect channel-map reads against runtime detach

[PATCH] ALSA: pcm: Protect channel-map reads against runtime detach

[Cen Zhang]

Channel-map controls remain accessible between PCM opens. In-tree HDA
code publishes the generic controls through add_std_chmaps(), so a
SNDRV_CTL_IOCTL_ELEM_READ can overlap the final close of the same PCM
substream.

pcm_chmap_ctl_get() checks substream->runtime and later reads its channel
count. Final close clears and frees that runtime under pcm->open_mutex,
while the control callback holds only controls_rwsem:

  Control GET                         Final PCM close
  -----------                         ---------------
  snd_ctl_elem_read()
    down_read(controls_rwsem)
    pcm_chmap_ctl_get()
      check substream->runtime
                                      lock(pcm->open_mutex)
                                      snd_pcm_release_substream()
                                        snd_pcm_detach_substream()
                                          substream->runtime = NULL
                                          kfree(runtime)
                                      unlock(pcm->open_mutex)
      substream->runtime->channels
        NULL or freed-pointer access
    up_read(controls_rwsem)

Reloading the cleared field can fault through NULL, while retaining the
old pointer can access freed storage. controls_rwsem keeps the control
alive, but does not serialize the matching PCM close.

Copy the channel count under open_mutex and use that scalar for the map
lookup. Use trylock with -EBUSY because snd_pcm_dev_disconnect() takes
open_mutex before removing controls under controls_rwsem; blocking in
the reverse order from GET could deadlock.

Fixes: 2d3391ec0ecc ("ALSA: PCM: channel mapping API implementation")
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>

---

diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
index 4d665b4148d7059402a55abb62fb24da8ff73f69..1ec7db324196f60d8c72a879c62ddae0a90ac0ff 100644
--- a/sound/core/pcm_lib.c
+++ b/sound/core/pcm_lib.c
@@ -2496,6 +2496,7 @@ static int pcm_chmap_ctl_get(struct snd_kcontrol *kcontrol,
 	unsigned int idx = snd_ctl_get_ioffidx(kcontrol, &ucontrol->id);
 	struct snd_pcm_substream *substream;
 	const struct snd_pcm_chmap_elem *map;
+	unsigned int channels;
 
 	if (!info->chmap)
 		return -EINVAL;
@@ -2504,11 +2505,20 @@ static int pcm_chmap_ctl_get(struct snd_kcontrol *kcontrol,
 		return -ENODEV;
 	memset(ucontrol->value.integer.value, 0,
 	       sizeof(long) * info->max_channels);
-	if (!substream->runtime)
-		return 0; /* no channels set */
+	/*
+	 * PCM disconnect takes open_mutex before removing controls.
+	 * Use trylock to avoid a deadlock with controls_rwsem.
+	 */
+	scoped_cond_guard(mutex_try, return -EBUSY,
+			  &info->pcm->open_mutex) {
+		if (!substream->runtime)
+			return 0; /* no channels set */
+		channels = substream->runtime->channels;
+	}
 	for (map = info->chmap; map->channels; map++) {
 		int i;
-		if (map->channels == substream->runtime->channels &&
+
+		if (map->channels == channels &&
 		    valid_chmap_channels(info, map->channels)) {
 			for (i = 0; i < map->channels; i++)
 				ucontrol->value.integer.value[i] = map->map[i];

Re: [PATCH] ALSA: pcm: Protect channel-map reads against runtime detach

[Takashi Iwai]

On Mon, 19 Oct 2026 12:29:33 +0200,
Cen Zhang wrote:
> 
> Channel-map controls remain accessible between PCM opens. In-tree HDA
> code publishes the generic controls through add_std_chmaps(), so a
> SNDRV_CTL_IOCTL_ELEM_READ can overlap the final close of the same PCM
> substream.
> 
> pcm_chmap_ctl_get() checks substream->runtime and later reads its channel
> count. Final close clears and frees that runtime under pcm->open_mutex,
> while the control callback holds only controls_rwsem:
> 
>   Control GET                         Final PCM close
>   -----------                         ---------------
>   snd_ctl_elem_read()
>     down_read(controls_rwsem)
>     pcm_chmap_ctl_get()
>       check substream->runtime
>                                       lock(pcm->open_mutex)
>                                       snd_pcm_release_substream()
>                                         snd_pcm_detach_substream()
>                                           substream->runtime = NULL
>                                           kfree(runtime)
>                                       unlock(pcm->open_mutex)
>       substream->runtime->channels
>         NULL or freed-pointer access
>     up_read(controls_rwsem)
> 
> Reloading the cleared field can fault through NULL, while retaining the
> old pointer can access freed storage. controls_rwsem keeps the control
> alive, but does not serialize the matching PCM close.
> 
> Copy the channel count under open_mutex and use that scalar for the map
> lookup. Use trylock with -EBUSY because snd_pcm_dev_disconnect() takes
> open_mutex before removing controls under controls_rwsem; blocking in
> the reverse order from GET could deadlock.
> 
> Fixes: 2d3391ec0ecc ("ALSA: PCM: channel mapping API implementation")
> Assisted-by: LLM
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> 
> ---
> 
> diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c
> index 4d665b4148d7059402a55abb62fb24da8ff73f69..1ec7db324196f60d8c72a879c62ddae0a90ac0ff 100644
> --- a/sound/core/pcm_lib.c
> +++ b/sound/core/pcm_lib.c
> @@ -2496,6 +2496,7 @@ static int pcm_chmap_ctl_get(struct snd_kcontrol *kcontrol,
>  	unsigned int idx = snd_ctl_get_ioffidx(kcontrol, &ucontrol->id);
>  	struct snd_pcm_substream *substream;
>  	const struct snd_pcm_chmap_elem *map;
> +	unsigned int channels;
>  
>  	if (!info->chmap)
>  		return -EINVAL;
> @@ -2504,11 +2505,20 @@ static int pcm_chmap_ctl_get(struct snd_kcontrol *kcontrol,
>  		return -ENODEV;
>  	memset(ucontrol->value.integer.value, 0,
>  	       sizeof(long) * info->max_channels);
> -	if (!substream->runtime)
> -		return 0; /* no channels set */
> +	/*
> +	 * PCM disconnect takes open_mutex before removing controls.
> +	 * Use trylock to avoid a deadlock with controls_rwsem.
> +	 */
> +	scoped_cond_guard(mutex_try, return -EBUSY,
> +			  &info->pcm->open_mutex) {
> +		if (!substream->runtime)
> +			return 0; /* no channels set */
> +		channels = substream->runtime->channels;
> +	}
>  	for (map = info->chmap; map->channels; map++) {
>  		int i;
> -		if (map->channels == substream->runtime->channels &&
> +
> +		if (map->channels == channels &&
>  		    valid_chmap_channels(info, map->channels)) {
>  			for (i = 0; i < map->channels; i++)
>  				ucontrol->value.integer.value[i] = map->map[i];

Taking a mutex from the kcontrol here sounds a bit scary, and
moreover, we need a saner and safer way to get the runtime reference
in general.  OTOH, for a workaround of this issue, this code change
looks  simple enough, so I took as is now.


thanks,

Takashi