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

[Cássio Gabriel]

On Sat, Mar 14, 2026 at 02:46:16PM +0100, Takashi Iwai wrote:
> On Sat, 14 Mar 2026 04:34:46 +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 can make an aliased ID pick the first entry with a matching
> > vid:pid even when that entry also depends on interface descriptor
> > fields that do not match the actual device or interface.
> > 
> > Fix it by re-checking each aliased candidate with usb_match_one_id()
> > against the real interface before returning the quirk.
> > 
> > Signed-off-by: Cássio Gabriel <cassiogabrielcontato@xxxxxxxxx>
> > ---
> >  sound/usb/card.c | 21 ++++++++++++++++-----
> >  1 file changed, 16 insertions(+), 5 deletions(-)
> > 
> > diff --git a/sound/usb/card.c b/sound/usb/card.c
> > index 270dad84d825..ff4418017763 100644
> > --- a/sound/usb/card.c
> > +++ b/sound/usb/card.c
> > @@ -870,17 +870,28 @@ static void find_last_interface(struct snd_usb_audio *chip)
> >  
> >  /* look for the corresponding quirk */
> >  static const struct snd_usb_audio_quirk *
> > -get_alias_quirk(struct usb_device *dev, unsigned int id)
> > +get_alias_quirk(struct usb_interface *intf, unsigned int id)
> >  {
> >  	const struct usb_device_id *p;
> > +	struct usb_device_id id_alias;
> >  
> >  	for (p = usb_audio_ids; p->match_flags; p++) {
> > -		/* FIXME: this checks only vendor:product pair in the list */
> >  		if ((p->match_flags & USB_DEVICE_ID_MATCH_DEVICE) ==
> >  		    USB_DEVICE_ID_MATCH_DEVICE &&
> >  		    p->idVendor == USB_ID_VENDOR(id) &&
> > -		    p->idProduct == USB_ID_PRODUCT(id))
> > -			return (const struct snd_usb_audio_quirk *)p->driver_info;
> > +		    p->idProduct == USB_ID_PRODUCT(id)) {
> > +			/*
> > +			 * Re-check the aliased entry against the actual
> > +			 * interface descriptors instead of matching only
> > +			 * the vendor/product pair.
> > +			 */
> > +			id_alias = *p;
> > +			id_alias.idVendor = USB_ID_VENDOR(id);
> > +			id_alias.idProduct = USB_ID_PRODUCT(id);
> > +
> > +			if (usb_match_one_id(intf, &id_alias))
> 
> Hmm, is this really a correct logic?
> 
> In this case, USB_ID_VENDOR(id) and USB_ID_PRODUCT(id) are very same
> as p->idVendor and p->idProduct, hence id_alias and *p are identical,
> i.e. you're basically comparing against *p.
> 
> I suppose you wanted to substitute the original device vendor/product
> IDs there instead?

Hi,

Yes, what I actually wanted was to keep the alias lookup by vid:pid, but
validate the additional match fields from the candidate entry against
the real device and interface descriptors.

I was thinking of keeping the initial lookup by the aliased vid:pid and
then explicitly checking only the remaining match bits, such as DEV_CLASS,
DEV_SUBCLASS, DEV_PROTOCOL, INT_CLASS, INT_SUBCLASS, INT_PROTOCOL, INT_NUMBER,
and bcdDevice_lo/hi. Another option would be to build the match object
from the original device ID instead of the aliased one.

I think the first option would be simpler and more direct here, since
the problem is that the current code resolves the alias only by
vendor/product, even though some quirk table entries also depend
on extra descriptor fields. The quirk table already has such entries,
for example USB_DEVICE_VENDOR_SPEC() and USB_AUDIO_DEVICE(), so explicitly
checking those extra fields in get_alias_quirk() seems to fit
the intended logic better.