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

[Takashi Iwai]

On Sat, 14 Mar 2026 19:22:41 +0100,
Cássio Gabriel wrote:
> 
> 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.

I don't have much opinion, you can compare both code and choose a
simpler one :)  In anyway, I'm waiting for your v2 patch.


thanks,

Takashi