Re: [PATCH v2] ALSA: usb-audio: validate full match when resolving quirk aliases

[Takashi Iwai]

On Tue, 17 Mar 2026 05:22:04 +0100,
Cássio Gabriel wrote:
> 
> get_alias_quirk() resolves a quirk for an aliased USB ID by scanning
> usb_audio_ids[], but it currently checks only the vendor/product pair.
> 
> This is weak for quirk table entries that also depend on additional
> USB_DEVICE_ID match fields, such as device or interface class,
> subclass, protocol, interface number, or bcdDevice range.
> 
> Rework the alias lookup so that it still uses the aliased vid:pid as
> the initial lookup key, but validates the remaining match_flags
> constraints of each candidate entry against the real device and
> interface descriptors before returning the quirk.
> 
> Signed-off-by: Cássio Gabriel <cassiogabrielcontato@xxxxxxxxx>
> ---
> Changes in v2:
> - drop the temporary usb_device_id reconstruction approach
> - validate only the remaining match_flags explicitly
> - pass struct usb_interface * to get_alias_quirk()
> - Link to v1: https://lore.kernel.org/r/20260314-alsa-usb-fix-quirk-alias-v1-1-3269998f7ada@xxxxxxxxx
> ---
>  sound/usb/card.c | 72 +++++++++++++++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 64 insertions(+), 8 deletions(-)
> 
> diff --git a/sound/usb/card.c b/sound/usb/card.c
> index fd81f32a66fb..153085a77d43 100644
> --- a/sound/usb/card.c
> +++ b/sound/usb/card.c
> @@ -864,21 +864,77 @@ static void find_last_interface(struct snd_usb_audio *chip)
>  	usb_audio_dbg(chip, "Found last interface = %d\n", chip->last_iface);
>  }
>  
> +/*
> + * Match aliased vid:pid first, then validate remaining fields against
> + * the real device and interface descriptors.
> + */
> +static bool snd_usb_match_alias_entry(struct usb_interface *intf,
> +				      const struct usb_device_id *id,
> +				      u32 alias_id)
> +{
> +	struct usb_device *dev = interface_to_usbdev(intf);
> +	const struct usb_host_interface *alt = intf->cur_altsetting;
> +	const struct usb_interface_descriptor *intfd = &alt->desc;
> +	const struct usb_device_descriptor *devd = &dev->descriptor;
> +	u16 bcd = le16_to_cpu(devd->bcdDevice);
> +
> +	/* Match aliased vendor/product */
> +	if ((id->match_flags & USB_DEVICE_ID_MATCH_VENDOR) &&
> +	    id->idVendor != USB_ID_VENDOR(alias_id))
> +		return false;
> +	if ((id->match_flags & USB_DEVICE_ID_MATCH_PRODUCT) &&
> +	    id->idProduct != USB_ID_PRODUCT(alias_id))
> +		return false;
> +	/* Match real device descriptor constraints */
> +	if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_LO) &&
> +	    bcd < id->bcdDevice_lo)
> +		return false;
> +	if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_HI) &&
> +	    bcd > id->bcdDevice_hi)
> +		return false;
> +	if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_CLASS) &&
> +	    devd->bDeviceClass != id->bDeviceClass)
> +		return false;
> +	if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_SUBCLASS) &&
> +	    devd->bDeviceSubClass != id->bDeviceSubClass)
> +		return false;
> +	if ((id->match_flags & USB_DEVICE_ID_MATCH_DEV_PROTOCOL) &&
> +	    devd->bDeviceProtocol != id->bDeviceProtocol)
> +		return false;
> +	/* Match real interface descriptor constraints */
> +	if ((id->match_flags & USB_DEVICE_ID_MATCH_INT_CLASS) &&
> +	    intfd->bInterfaceClass != id->bInterfaceClass)
> +		return false;
> +	if ((id->match_flags & USB_DEVICE_ID_MATCH_INT_SUBCLASS) &&
> +	    intfd->bInterfaceSubClass != id->bInterfaceSubClass)
> +		return false;
> +	if ((id->match_flags & USB_DEVICE_ID_MATCH_INT_PROTOCOL) &&
> +	    intfd->bInterfaceProtocol != id->bInterfaceProtocol)
> +		return false;
> +	if ((id->match_flags & USB_DEVICE_ID_MATCH_INT_NUMBER) &&
> +	    intfd->bInterfaceNumber != id->bInterfaceNumber)
> +		return false;
> +
> +	return true;
> +}

Hrm, it became larger than I wished.
You compared with another implementation and decided to choose this
version?

An alternative would be to have a copy of usb_device_id like your
previous version, and clear match_flags bits with
~USB_DEVICE_ID_MATCH_DEVICE.  If match_flags becomes 0, it passes.
Otherwise call usb_match_one_id().


thanks,

Takashi