Re: [PATCH v2 07/21] ASoC: ops: Introduce 'soc_set_enum_kctl'

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ASoC: snd_soc_set_enum_kctl passes uninitialized control ID to callbacks
- [Low] ASoC: soc_set_enum_kctl allocates massive ALSA structures on kernel stack
--

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

ASoC: ops: Introduce 'soc_set_enum_kctl'

The new function is to be used to set enumerated controls to desired
values. It is something a machine driver may call in fixup_controls.

> diff --git a/sound/soc/soc-ops.c b/sound/soc/soc-ops.c
> index 4d799c5e44f1b..b1eb74e241932 100644
> --- a/sound/soc/soc-ops.c
> +++ b/sound/soc/soc-ops.c

[ ... ]

> @@ -505,6 +505,75 @@ int snd_soc_deactivate_kctl(struct snd_soc_card *card,
>  }
>  EXPORT_SYMBOL_GPL(snd_soc_deactivate_kctl);
>  
> +static int soc_set_enum_kctl(struct snd_kcontrol *kctl, const char *value)
> +{
> +	struct snd_ctl_elem_value evalue = { 0 };
> +	struct snd_ctl_elem_info info = { 0 };

[Severity: Low]
Are these ALSA structures too large to be allocated on the stack here?

struct snd_ctl_elem_value is typically around 1220 bytes on 64-bit
architectures since it contains unions with large fixed-size arrays. With
struct snd_ctl_elem_info adding another 272 bytes, allocating both on the
stack consumes approximately 1.5 KB of stack space.

Could this exceed the 1024-byte FRAME_WARN threshold on some architectures
and increase the risk of a kernel stack overflow?

The ALSA core usually relies on heap allocation for these large structures
to avoid exhausting the stack.

> +	int sel, i, ret;
> +
> +	ret = kctl->info(kctl, &info);
> +	if (ret < 0)
> +		return ret;

[ ... ]

> +	for (i = 0; i < info.count; i++)
> +		evalue.value.enumerated.item[i] = sel;
> +
> +	if (kctl->put)
> +		return kctl->put(kctl, &evalue);

[Severity: High]
Does this pass an uninitialized control ID to the put callback?

The evalue and info structures are zero-initialized above, which leaves
their id fields completely empty. The kctl->id is never copied into them
before being passed to kctl->info() or kctl->put().

The core ALSA API guarantees that ucontrol->id is fully populated before
invoking control callbacks. Some drivers rely on this contract. For example,
rt700_mux_put() uses the control name to identify which specific control
is being accessed:

sound/soc/codecs/rt700.c:rt700_mux_put() {
    ...
    if (strstr(ucontrol->id.name, "HPO Mux"))
        ...
}

Because evalue.id.name is an empty string here, will such callbacks fail
or execute incorrect logic?

> +	else
> +		return -EINVAL;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261004-macaudio-v2-0-7b47ed6335a8@gmail.com?part=7