Re: [RFC PATCH 13/16] ALSA: mixer_audient: Add enumerated controls

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] ALSA: mixer_audient: unselectable enum item in
  snd_ctl_audient_info() breaks UAPI
--

commit 11b395622645c1753139b078b4521e003601a600
Author: Will Porter <mrwillporter@gmail.com>

ALSA: mixer_audient: Add enumerated controls

This commit adds support for enumerated mixer controls in the Audient USB
mixer driver, including a trailing read-only item for unknown or unconfirmed
device values.

> diff --git a/sound/usb/mixer_audient.c b/sound/usb/mixer_audient.c
> index c4ce2340f8bba..3e6cd8981b4f1 100644
> --- a/sound/usb/mixer_audient.c
> +++ b/sound/usb/mixer_audient.c

[ ... ]

> @@ -560,12 +582,116 @@ static int snd_ctl_audient_integer_set(struct snd_kcontrol *kctl,

[ ... ]

> +static int snd_ctl_audient_enum_set(struct snd_kcontrol *kctl,
> +				    struct snd_ctl_elem_value *ctl_val)
> +{
> +	struct usb_mixer_elem_info *const elem = snd_kcontrol_chip(kctl);
> +	struct usb_mixer_interface *const mixer = elem->head.mixer;
> +
> +	const unsigned int hwchan = snd_audient_kctl_hwchannels(kctl);
> +	const struct snd_audient_ctrl_type *const ctype
> +						= snd_audient_kctl_type(kctl);
> +	const struct snd_audient_enum *const e = ctype->enumeration;
> +	const u8 base = snd_audient_kctl_cidxoffs(kctl);
> +
> +	unsigned int i;
> +	int changed = 0;
> +
> +	for (i = 0; i < hwchan; i++)
> +		if (ctl_val->value.enumerated.item[i] >= e->items)
> +			return -EINVAL;

[ ... ]

>  static int snd_ctl_audient_info(struct snd_kcontrol *kctl,
>  			    struct snd_ctl_elem_info *uinfo)
>  {
>  	const struct snd_audient_ctrl_type *const ctype
>  						= snd_audient_kctl_type(kctl);
>  
> +	if (ctype->type == SNDRV_CTL_ELEM_TYPE_ENUMERATED)
> +		return snd_ctl_enum_info(uinfo, snd_audient_kctl_channels(kctl),
> +					 ctype->enumeration->items + 1,
> +					 ctype->enumeration->texts);

[Severity: Medium]
Does advertising ctype->enumeration->items + 1 items in snd_ctl_audient_info()
while rejecting item index e->items with -EINVAL in snd_ctl_audient_enum_set()
break the ALSA control UAPI contract for writable enumerated controls?

For a SNDRV_CTL_ELEM_ACCESS_READWRITE enumerated control, userspace expects
all advertised item indices (0 .. uinfo->value.enumerated.items - 1) to be
valid on write. Rejecting the last advertised item causes the ALSA mixer-test
kselftest (test_ctl_write_valid_enumerated() and test_ctl_write_side_effects())
to fail on every enumerated control, and breaks alsactl store/restore or
multi-channel read-modify-write updates whenever snd_ctl_audient_enum_get()
returns e->items.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106163129.1006907-1-mrwillporter@gmail.com?part=13