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

[Cássio Gabriel]

On Tue, Mar 17, 2026 at 10:03:10AM +0100, Takashi Iwai wrote:
> 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?
> 

Hi,

I went with the explicit matching path in v2 because I wanted to keep
the alias lookup based on the aliased vid:pid while validating the
remaining match_flags against the real descriptors.

> 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, that makes sense.

What do you think about this approach?
```
static const struct snd_usb_audio_quirk *
get_alias_quirk(struct usb_interface *intf, unsigned int id)
{
        const struct usb_device_id *p;
        struct usb_device_id match_id;

        for (p = usb_audio_ids; p->match_flags; p++) {
                if ((p->match_flags & USB_DEVICE_ID_MATCH_DEVICE) !=
                    USB_DEVICE_ID_MATCH_DEVICE)
                        continue;

                if (p->idVendor != USB_ID_VENDOR(id) ||
                    p->idProduct != USB_ID_PRODUCT(id))
                        continue;

                match_id = *p;
                match_id.match_flags &= ~USB_DEVICE_ID_MATCH_DEVICE;
                if (!match_id.match_flags || usb_match_one_id(intf, &match_id))
                        return (const struct snd_usb_audio_quirk *)
                                p->driver_info;
        }

        return NULL;
}
```

Thanks,
Cássio.