[PATCH] USB: fix gathering of interface associations

[PATCH] USB: fix gathering of interface associations

[Daniel Mack]

TEAC's UD-H01 (and probably other devices) have a gap in the interface
number allocation of their descriptors:

  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength          220
    bNumInterfaces          3
    [...]
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      [...]
    Interface Association:
      bLength                 8
      bDescriptorType        11
      bFirstInterface         2
      bInterfaceCount         2
      bFunctionClass          1 Audio
      bFunctionSubClass       0
      bFunctionProtocol      32
      iFunction               4
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        2
      bAlternateSetting       0
      [...]

Once a configuration is selected, usb_set_configuration() walks the
known interfaces of a given configuration and calls find_iad() on
each of them to set the interface association pointer the interface
is included in.

The problem here is that the loop variable is taken for the interface
number in the comparison logic that gathers the association. Which is
fine as long as the descriptors are sane.

In the case above, however, the logic gets out of sync and the
interface association fields of all interfaces beyond the interface
number gap are wrong.

Fix this by passing the interface's bInterfaceNumber to find_iad()
instead.

Signed-off-by: Daniel Mack <zonque@gmail.com>
Reported-by: bEN <ml_all@circa.be>
Reported-by: Ivan Perrone <ivanperrone@hotmail.com>
---

This patch has been tested on a virtual USB device emulated in QEMU.
I'm still waiting for test results based on real hardware, but the bug
is rather obvious is and the fix really simple.

Daniel

 drivers/usb/core/message.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index b548cf1..bdd1c67 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1838,7 +1838,6 @@ free_interfaces:
 		intfc = cp->intf_cache[i];
 		intf->altsetting = intfc->altsetting;
 		intf->num_altsetting = intfc->num_altsetting;
-		intf->intf_assoc = find_iad(dev, cp, i);
 		kref_get(&intfc->ref);
 
 		alt = usb_altnum_to_altsetting(intf, 0);
@@ -1851,6 +1850,8 @@ free_interfaces:
 		if (!alt)
 			alt = &intf->altsetting[0];
 
+		intf->intf_assoc =
+			find_iad(dev, cp, alt->desc.bInterfaceNumber);
 		intf->cur_altsetting = alt;
 		usb_enable_interface(dev, intf, true);
 		intf->dev.parent = &dev->dev;
-- 
1.7.10.2

Re: [PATCH] USB: fix gathering of interface associations

[Daniel Mack]

[-- Attachment #1: Type: text/plain, Size: 2329 bytes --]

On 12.06.2012 20:23, Daniel Mack wrote:
> TEAC's UD-H01 (and probably other devices) have a gap in the interface
> number allocation of their descriptors:

I now got a report from Sergio Tabanelli who tried this patch on real
hardware, and he confirmed the fix. I added some more people to the
patch description and copied stable@ as well.

Greg, can can this go through your tree?


Daniel


> 
>   Configuration Descriptor:
>     bLength                 9
>     bDescriptorType         2
>     wTotalLength          220
>     bNumInterfaces          3
>     [...]
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        0
>       bAlternateSetting       0
>       [...]
>     Interface Association:
>       bLength                 8
>       bDescriptorType        11
>       bFirstInterface         2
>       bInterfaceCount         2
>       bFunctionClass          1 Audio
>       bFunctionSubClass       0
>       bFunctionProtocol      32
>       iFunction               4
>     Interface Descriptor:
>       bLength                 9
>       bDescriptorType         4
>       bInterfaceNumber        2
>       bAlternateSetting       0
>       [...]
> 
> Once a configuration is selected, usb_set_configuration() walks the
> known interfaces of a given configuration and calls find_iad() on
> each of them to set the interface association pointer the interface
> is included in.
> 
> The problem here is that the loop variable is taken for the interface
> number in the comparison logic that gathers the association. Which is
> fine as long as the descriptors are sane.
> 
> In the case above, however, the logic gets out of sync and the
> interface association fields of all interfaces beyond the interface
> number gap are wrong.
> 
> Fix this by passing the interface's bInterfaceNumber to find_iad()
> instead.
> 
> Signed-off-by: Daniel Mack <zonque-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Reported-by: bEN <ml_all-+8pbQKgpUcE@public.gmane.org>
> Reported-by: Ivan Perrone <ivanperrone-PkbjNfxxIARBDgjK7y7TUQ@public.gmane.org>
> ---
> 
> This patch has been tested on a virtual USB device emulated in QEMU.
> I'm still waiting for test results based on real hardware, but the bug
> is rather obvious is and the fix really simple.
> 
> Daniel

[-- Attachment #2: 0001-USB-fix-gathering-of-interface-associations.patch --]
[-- Type: text/x-patch, Size: 2948 bytes --]

>From bc5818593a73dfe895b329cd5f6c95b72d641e9e Mon Sep 17 00:00:00 2001
From: Daniel Mack <zonque-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Date: Sun, 10 Jun 2012 22:31:15 +0200
Subject: [PATCH] USB: fix gathering of interface associations

TEAC's UD-H01 (and probably other devices) have a gap in the interface
number allocation in their descriptors:

  Configuration Descriptor:
    bLength                 9
    bDescriptorType         2
    wTotalLength          220
    bNumInterfaces          3
    [...]
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        0
      bAlternateSetting       0
      [...]
    Interface Association:
      bLength                 8
      bDescriptorType        11
      bFirstInterface         2
      bInterfaceCount         2
      bFunctionClass          1 Audio
      bFunctionSubClass       0
      bFunctionProtocol      32
      iFunction               4
    Interface Descriptor:
      bLength                 9
      bDescriptorType         4
      bInterfaceNumber        2
      bAlternateSetting       0
      [...]

Once a configuration is selected on this device, usb_set_configuration()
walks the known interfaces of a given configuration and calls find_iad()
on each of them to set the interface association pointer the interface
is included in.

The problem here is that the loop varibale index is taken for the
interface number in the comparison logic that gathers the association.
Which is fine as long as the descriptors are sane.

In the case above, however, the logic gets out of sync and the
interface association fields of all interfaces beyond the interface
number gap are wrong.

Fix this by passing the interface's bInterfaceNumber to find_iad()
instead.

Signed-off-by: Daniel Mack <zonque-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Reported-by: bEN <ml_all-+8pbQKgpUcE@public.gmane.org>
Reported-by: Ivan Perrone <ivanperrone-PkbjNfxxIARBDgjK7y7TUQ@public.gmane.org>
Tested-by: Sergio Tabanelli <sergio.tabanelli-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Cc: <stable-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 drivers/usb/core/message.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index b548cf1..bdd1c67 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1838,7 +1838,6 @@ free_interfaces:
 		intfc = cp->intf_cache[i];
 		intf->altsetting = intfc->altsetting;
 		intf->num_altsetting = intfc->num_altsetting;
-		intf->intf_assoc = find_iad(dev, cp, i);
 		kref_get(&intfc->ref);
 
 		alt = usb_altnum_to_altsetting(intf, 0);
@@ -1851,6 +1850,8 @@ free_interfaces:
 		if (!alt)
 			alt = &intf->altsetting[0];
 
+		intf->intf_assoc =
+			find_iad(dev, cp, alt->desc.bInterfaceNumber);
 		intf->cur_altsetting = alt;
 		usb_enable_interface(dev, intf, true);
 		intf->dev.parent = &dev->dev;
-- 
1.7.10.2

Re: [PATCH] USB: fix gathering of interface associations

[Benjamin Sion]

On 14/06/12 13:15, Daniel Mack wrote:
> On 12.06.2012 20:23, Daniel Mack wrote:
>> TEAC's UD-H01 (and probably other devices) have a gap in the interface
>> number allocation of their descriptors:
> 
> I now got a report from Sergio Tabanelli who tried this patch on real
> hardware, and he confirmed the fix. I added some more people to the
> patch description and copied stable@ as well.

Daniel,

just for information, I only can try the patch on real hardware (TEAC
UD-H01) during this week-end.

btw, thanks a lot for your effort on this patch.

bEN

Re: [PATCH] USB: fix gathering of interface associations

[Greg KH]

On Thu, Jun 14, 2012 at 01:15:30PM +0200, Daniel Mack wrote:
> On 12.06.2012 20:23, Daniel Mack wrote:
> > TEAC's UD-H01 (and probably other devices) have a gap in the interface
> > number allocation of their descriptors:
> 
> I now got a report from Sergio Tabanelli who tried this patch on real
> hardware, and he confirmed the fix. I added some more people to the
> patch description and copied stable@ as well.
> 
> Greg, can can this go through your tree?

Yup, I'll take it now, thanks.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] USB: fix gathering of interface associations

[Benjamin Sion]

On 14/06/12 13:15, Daniel Mack wrote:
> On 12.06.2012 20:23, Daniel Mack wrote:
>> TEAC's UD-H01 (and probably other devices) have a gap in the interface
>> number allocation of their descriptors:
> 
> I now got a report from Sergio Tabanelli who tried this patch on real
> hardware, and he confirmed the fix. I added some more people to the
> patch description and copied stable@ as well.

Your patch just tested here on real hardware (TEAC UD-H01), I confirm it
works: interface is registered (despite UD-H01's buggy descriptors).

Thanks!

bEN