public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [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