* [PATCH] ALSA: usb-audio: validate full match when resolving quirk aliases @ 2026-03-14 3:34 Cássio Gabriel 2026-03-14 13:46 ` Takashi Iwai 0 siblings, 1 reply; 4+ messages in thread From: Cássio Gabriel @ 2026-03-14 3:34 UTC (permalink / raw) To: Takashi Iwai Cc: Jaroslav Kysela, Mark Brown, Greg Kroah-Hartman, linux-sound, linux-usb, linux-kernel, Cássio Gabriel 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@gmail.com> --- 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)) + return (const struct snd_usb_audio_quirk *)p->driver_info; + } } return NULL; @@ -931,7 +942,7 @@ static int usb_audio_probe(struct usb_interface *intf, id = USB_ID(le16_to_cpu(dev->descriptor.idVendor), le16_to_cpu(dev->descriptor.idProduct)); if (get_alias_id(dev, &id)) - quirk = get_alias_quirk(dev, id); + quirk = get_alias_quirk(intf, id); if (quirk && quirk->ifnum >= 0 && ifnum != quirk->ifnum) return -ENXIO; if (quirk && quirk->ifnum == QUIRK_NODEV_INTERFACE) --- base-commit: 1f318b96cc84d7c2ab792fcc0bfd42a7ca890681 change-id: 20260313-alsa-usb-fix-quirk-alias-e9c8300ba836 Best regards, -- Cássio Gabriel <cassiogabrielcontato@gmail.com> ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] ALSA: usb-audio: validate full match when resolving quirk aliases 2026-03-14 3:34 [PATCH] ALSA: usb-audio: validate full match when resolving quirk aliases Cássio Gabriel @ 2026-03-14 13:46 ` Takashi Iwai 2026-03-14 18:22 ` Cássio Gabriel 0 siblings, 1 reply; 4+ messages in thread From: Takashi Iwai @ 2026-03-14 13:46 UTC (permalink / raw) To: Cássio Gabriel Cc: Takashi Iwai, Jaroslav Kysela, Mark Brown, Greg Kroah-Hartman, linux-sound, linux-usb, linux-kernel 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@gmail.com> > --- > 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? thanks, Takashi ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ALSA: usb-audio: validate full match when resolving quirk aliases 2026-03-14 13:46 ` Takashi Iwai @ 2026-03-14 18:22 ` Cássio Gabriel 2026-03-16 16:37 ` Takashi Iwai 0 siblings, 1 reply; 4+ messages in thread From: Cássio Gabriel @ 2026-03-14 18:22 UTC (permalink / raw) To: Takashi Iwai Cc: Takashi Iwai, Jaroslav Kysela, Mark Brown, Greg Kroah-Hartman, linux-sound, linux-usb, linux-kernel 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@gmail.com> > > --- > > 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. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] ALSA: usb-audio: validate full match when resolving quirk aliases 2026-03-14 18:22 ` Cássio Gabriel @ 2026-03-16 16:37 ` Takashi Iwai 0 siblings, 0 replies; 4+ messages in thread From: Takashi Iwai @ 2026-03-16 16:37 UTC (permalink / raw) To: Cássio Gabriel Cc: Takashi Iwai, Takashi Iwai, Jaroslav Kysela, Mark Brown, Greg Kroah-Hartman, linux-sound, linux-usb, linux-kernel 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@gmail.com> > > > --- > > > 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 ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-03-16 16:37 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-03-14 3:34 [PATCH] ALSA: usb-audio: validate full match when resolving quirk aliases Cássio Gabriel 2026-03-14 13:46 ` Takashi Iwai 2026-03-14 18:22 ` Cássio Gabriel 2026-03-16 16:37 ` Takashi Iwai
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox