Re: [PATCH 10/28] ASoC: ops: Accept patterns in snd_soc_limit_volume

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] ASoC: ops: Data race and UAF in snd_soc_limit_volume list traversal
- [High] ASoC: ops: Type confusion and NULL pointer dereference in soc_limit_volume
- [High] ASoC: ops: snd_soc_limit_volume API breakage causes audio initialization failure
--

commit 71061718845772d7571d727c356dc0cbae99856e
Author: Martin Povišer <povik+lin@cutebit.org>

ASoC: ops: Accept patterns in snd_soc_limit_volume

This patch modifies snd_soc_limit_volume() to accept wildcard patterns,
allowing it to apply limits to multiple matching controls at once. It also
returns the number of matching controls on success instead of 0.

> diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c
> index e572b05840b48..2f91ee7946bb1 100644
> --- a/sound/soc/soc-ops.c
> +++ b/sound/soc/soc-ops.c
[ ... ]
> @@ -445,31 +468,47 @@ static int soc_limit_volume(struct snd_kcontrol *kctl, int max)
>  }
>  
>  /**
> - * snd_soc_limit_volume - Set new limit to an existing volume control.
> + * snd_soc_limit_volume - Set new limit to existing volume controls.
>   *
>   * @card: where to look for the control
> - * @name: Name of the control
> + * @name: name pattern
>   * @max: new maximum limit
>   *
> - * Return 0 for success, else error.
> + * Finds controls matching the given name (which can be either a name
> + * verbatim, or a pattern starting with the wildcard '*') and sets
> + * a platform volume limit on them.
> + *
> + * Return number of matching controls on success, else error. At least
> + * one control needs to match the pattern.
>   */
>  int snd_soc_limit_volume(struct snd_soc_card *card, const char *name, int max)
>  {
>  	struct snd_kcontrol *kctl;
> +	int hits = 0;
> +	int ret;
>  
>  	/* Sanity check for name */
>  	if (unlikely(!name))
>  		return -EINVAL;
>  
> -	kctl = snd_soc_card_get_kcontrol(card, name);
> -	if (!kctl) {
> +	list_for_each_entry(kctl, &card->snd_card->controls, list) {

[Severity: High]
Does this locklessly traverse the ALSA control list?

If a local user dynamically adds or removes ALSA user controls via
SNDRV_CTL_IOCTL_ELEM_ADD or SNDRV_CTL_IOCTL_ELEM_REMOVE, couldn't traversing
the list here without holding card->controls_rwsem or card->controls_rwlock
result in memory corruption and a kernel panic?

> +		if (!soc_control_matches(kctl, name))
> +			continue;
> +
> +		ret = soc_limit_volume(kctl, max);

[Severity: High]
Since the wildcard matching can now match dynamically created user ALSA
controls, is it safe to pass kctl directly to soc_limit_volume() without
verifying it is actually a mixer control?

For user controls, kctl->private_value is NULL. When soc_limit_volume()
unconditionally casts this to a struct soc_mixer_control pointer and
accesses mc->max or mc->platform_max:

soc_limit_volume(struct snd_kcontrol *kctl, int max)
{
	struct soc_mixer_control *mc =
		(struct soc_mixer_control *)kctl->private_value;

	if (max <= 0 || max > mc->max - mc->min)
		...

won't this result in a NULL pointer dereference?

> +		if (ret < 0)
> +			return ret;
> +		hits++;
> +	}
> +
> +	if (!hits) {
>  		/* Some cards blindly add limits for multiple variants. */
>  		dev_dbg(card->dev, "Volume limit for unknown control '%s'\n",
>  			name);
>  		return -EINVAL;
>  	}
>  
> -	return soc_limit_volume(kctl, max);
> +	return hits;

[Severity: High]
Could changing the success return value from 0 to a positive integer break
existing callers?

Callers like asoc_sdw_cs35l56_volume_limit() interpret any non-zero return
value as an error, propagating it and bypassing further initialization steps
like snd_soc_dapm_add_routes().

>  }
>  EXPORT_SYMBOL_GPL(snd_soc_limit_volume);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260920-macaudio-v1-0-741cc20a74e5@gmail.com?part=10