usb: gadget: composite: Fix NULL pointer crash in USB compliance test

PATCH New Stefan Roese 3 participants 5 messages Jan 9, 2015 – Jan 10, 2015 USB

Share & tools

Stefan Roese

author

2015-01-09 13:54 UTC #0 lore raw

PATCH 1/2 usb: gadget: composite: Fix NULL pointer crash in USB compliance test

On the DXR2 board (AM335x using MUSB) the USB compliance test suite (USB 2.0 Command Verifier) will cause the board to crash and reset upon the "BOS Descriptor Test - Addressed state". Here the output from the DRX2 while running this test:

GADGET DRIVER: usb_dnl_dfu
musb-hdrc: peripheral reset irq lost!
composite_setup (776)
data abort

pc : [<87f693ac>]          lr : [<87f6911c>]

sp : 86f33a58 ip : 00000000 fp : 86f3bbac
r10: 00000f00 r9 : 86f33ef4 r8 : 86f37da8
r7 : 00000005 r6 : 86f33a90 r5 : 00000000 r4 : 86f37e30
r3 : 00000000 r2 : 00000000 r1 : 87f9c888 r0 : 00000016
Flags: Nzcv IRQs off FIQs on Mode SVC_32
Resetting CPU ...

resetting ...

By adding the case statement for USB_DT_BOS and therefore not running into the default case (jump to unkown label) this crash is fixed.

Signed-off-by Stefan Roese <[email protected]>

Cc Roger Meier <[email protected]>

Cc Samuel Egli <[email protected]>

Cc Enrico Leto <[email protected]>

Cc Heiko Schocher <[email protected]>

Cc Lukasz Majewski <[email protected]>

Cc Marek Vasut <[email protected]>

---
 drivers/usb/gadget/composite.c | 8 ++++++++
 1 file changed, 8 insertions(+)

drivers/usb/gadget/composite.c +8 -0

index a4c5606..98c2da6 100644
@@ -761,6 +761,14 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 			if (value >= 0)
 				value = min(w_length, (u16) value);
 			break;
+		case USB_DT_BOS:
+			/*
+			 * The USB compliance test (USB 2.0 Command Verifier)
+			 * issues this request. We should not run into the
+			 * default path here. But return for now until
+			 * the superspeed support is added.
+			 */
+			break;
 		default:
 			goto unknown;
 		}

Signature · 1 line

2.2.1

Stefan Roese

author

2015-01-09 13:54 UTC #1 lore raw

PATCH 2/2 usb: gadget: f_dfu: Add get_alt function to pass the USB compliance test

Without this function the USB compliance test (USB 2.0 Command Verifier) will fail in the "Interface Descriptor Test" with this error message:

FAIL
(1.2.51) A successful GetInterface request must return the alternate setting set by a prior call to SetInterface.

Lets add this function to read back the value so that the DFU device fully passes the USB compliance test.

Signed-off-by Stefan Roese <[email protected]>

Cc Roger Meier <[email protected]>

Cc Samuel Egli <[email protected]>

Cc Enrico Leto <[email protected]>

Cc Heiko Schocher <[email protected]>

Cc Lukasz Majewski <[email protected]>

Cc Marek Vasut <[email protected]>

---
 drivers/usb/gadget/f_dfu.c | 8 ++++++++
 1 file changed, 8 insertions(+)

drivers/usb/gadget/f_dfu.c +8 -0

index ead71eb..77a1567 100644
@@ -780,6 +780,13 @@ static int dfu_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
 	return 0;
 }
 
+static int __dfu_get_alt(struct usb_function *f, unsigned intf)
+{
+	struct f_dfu *f_dfu = func_to_dfu(f);
+
+	return f_dfu->altsetting;
+}
+
 /* TODO: is this really what we need here? */
 static void dfu_disable(struct usb_function *f)
 {

@@ -806,6 +813,7 @@ static int dfu_bind_config(struct usb_configuration *c)
 	f_dfu->usb_function.bind = dfu_bind;
 	f_dfu->usb_function.unbind = dfu_unbind;
 	f_dfu->usb_function.set_alt = dfu_set_alt;
+	f_dfu->usb_function.get_alt = __dfu_get_alt;
 	f_dfu->usb_function.disable = dfu_disable;
 	f_dfu->usb_function.strings = dfu_generic_strings;
 	f_dfu->usb_function.setup = dfu_handle;

Signature · 1 line

2.2.1

Heiko Schocher

2015-01-09 13:59 UTC #2 lore raw

PATCH 2/2 usb: gadget: f_dfu: Add get_alt function to pass the USB compliance test

Hello Stefan,

Am 09.01.2015 14:54, schrieb Stefan Roese:

Quoted 17 lines

Without this function the USB compliance test (USB 2.0 Command Verifier) will fail in the "Interface Descriptor Test" with this error message:
FAIL
(1.2.51) A successful GetInterface request must return the alternate setting set by a prior call to SetInterface.
Lets add this function to read back the value so that the DFU device fully passes the USB compliance test.
Signed-off-by Stefan Roese <[email protected]>
Cc Roger Meier <[email protected]>
Cc Samuel Egli <[email protected]>
Cc Enrico Leto <[email protected]>
Cc Heiko Schocher <[email protected]>
Cc Lukasz Majewski <[email protected]>
Cc Marek Vasut <[email protected]>

Quoted 3 lines

---
  drivers/usb/gadget/f_dfu.c | 8 ++++++++
  1 file changed, 8 insertions(+)

Thanks!

Acked-by Heiko Schocher <[email protected]>

bye,
Heiko

Signature · 2 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Heiko Schocher

2015-01-09 13:59 UTC #3 lore raw

PATCH 1/2 usb: gadget: composite: Fix NULL pointer crash in USB compliance test

In reply to Stefan Roese

Hello Stefan,

Am 09.01.2015 14:54, schrieb Stefan Roese:

Quoted 29 lines

On the DXR2 board (AM335x using MUSB) the USB compliance test suite (USB 2.0 Command Verifier) will cause the board to crash and reset upon the "BOS Descriptor Test - Addressed state". Here the output from the DRX2 while running this test:
GADGET DRIVER: usb_dnl_dfu
musb-hdrc: peripheral reset irq lost!
composite_setup (776)
data abort
pc : [<87f693ac>]          lr : [<87f6911c>]
sp : 86f33a58 ip : 00000000 fp : 86f3bbac
r10: 00000f00 r9 : 86f33ef4 r8 : 86f37da8
r7 : 00000005 r6 : 86f33a90 r5 : 00000000 r4 : 86f37e30
r3 : 00000000 r2 : 00000000 r1 : 87f9c888 r0 : 00000016
Flags: Nzcv IRQs off FIQs on Mode SVC_32
Resetting CPU ...
resetting ...
By adding the case statement for USB_DT_BOS and therefore not running into the default case (jump to unkown label) this crash is fixed.
Signed-off-by Stefan Roese <[email protected]>
Cc Roger Meier <[email protected]>
Cc Samuel Egli <[email protected]>
Cc Enrico Leto <[email protected]>
Cc Heiko Schocher <[email protected]>
Cc Lukasz Majewski <[email protected]>
Cc Marek Vasut <[email protected]>

Quoted 3 lines

---
  drivers/usb/gadget/composite.c | 8 ++++++++
  1 file changed, 8 insertions(+)

Thanks!

Acked-by Heiko Schocher <[email protected]>

bye,
Heiko

Quoted 21 lines

drivers/usb/gadget/composite.c +8 -0

index a4c5606..98c2da6 100644
@@ -761,6 +761,14 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
  			if (value >= 0)
  				value = min(w_length, (u16) value);
  			break;
+		case USB_DT_BOS:
+			/*
+			 * The USB compliance test (USB 2.0 Command Verifier)
+			 * issues this request. We should not run into the
+			 * default path here. But return for now until
+			 * the superspeed support is added.
+			 */
+			break;
  		default:
  			goto unknown;
  		}

Signature · 2 lines

DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany

Marek Vasut

Applied

2015-01-10 00:47 UTC #4 lore raw

PATCH 1/2 usb: gadget: composite: Fix NULL pointer crash in USB compliance test

In reply to Stefan Roese

Quoted 17 lines · Stefan Roese

On the DXR2 board (AM335x using MUSB) the USB compliance test suite (USB 2.0 Command Verifier) will cause the board to crash and reset upon the "BOS Descriptor Test - Addressed state". Here the output from the DRX2 while running this test:
GADGET DRIVER: usb_dnl_dfu
musb-hdrc: peripheral reset irq lost!
composite_setup (776)
data abort
pc : [<87f693ac>]          lr : [<87f6911c>]
sp : 86f33a58 ip : 00000000 fp : 86f3bbac
r10: 00000f00 r9 : 86f33ef4 r8 : 86f37da8
r7 : 00000005 r6 : 86f33a90 r5 : 00000000 r4 : 86f37e30
r3 : 00000000 r2 : 00000000 r1 : 87f9c888 r0 : 00000016
Flags: Nzcv IRQs off FIQs on Mode SVC_32
Resetting CPU ...

Quoted 4 lines

resetting ...
By adding the case statement for USB_DT_BOS and therefore not running into the default case (jump to unkown label) this crash is fixed.

Applied both, thank you!

Best regards,
Marek Vasut