The set_config() function iterates over all USB composite
function's descriptors in search for endpoint descriptors.
It, however, fails to verify whether USB composite function
has any descriptors.
Signed-off-by: Michal Nazarewicz <m.nazarewicz@xxxxxxxxxxx>
Reported-by: Holger Brunck <holger.brunck@xxxxxxxxxxx>
---
drivers/usb/gadget/composite.c | 43 +++++++++++++++++++++++----------------
1 files changed, 25 insertions(+), 18 deletions(-)
On Fri, 24 Sep 2010 15:39:01 +0200, Holger brunck wrote:
> we use a USB gadget driver on a MPC8247 based board. We use the connection for
> serial emulation. In the past we used Kernel 2.6.28. During our upgrade to
> 2.6.33 we see a kernel crash if we plug in the USB connector.
> After investigating the code I saw that this was introduced due to the commit
> USB gadget: Handle endpoint requests at the function level
> I made a patch which solves the problem for our board. It can be seen below.
> Maybe this null pointer check should also be done in the mainline kernel.
> diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
> @@ -436,7 +436,7 @@ static int set_config(struct usb_composite_dev *cdev,
> else
> descriptors = f->descriptors;
>
> - for (; *descriptors; ++descriptors) {
> + for (; descriptors && *descriptors; ++descriptors) {
> struct usb_endpoint_descriptor *ep;
> int addr;
IMO putting this check into the for test clause is not the cleanest: what's
the point of checking it before each iteration? I think the check should be
done one before the loop as shown by attached patch.
diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index 1160c55..90575d7 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -379,6 +379,30 @@ static void reset_config(struct usb_composite_dev *cdev)
cdev->config = NULL;
}
+static void record_endpoints(struct usb_function *f,
+ enum usb_device_speed speed)
+{
+ struct usb_descriptor_header **descriptors;
+ struct usb_endpoint_descriptor *ep;
+ int addr;
+
+ if (speed == USB_SPEED_HIGH)
+ descriptors = f->hs_descriptors;
+ else
+ descriptors = f->descriptors;
+
+ if (!descriptors)
+ return;
+
+ for (; *descriptors; ++descriptors)
+ if ((*descriptors)->bDescriptorType == USB_DT_ENDPOINT) {
+ ep = (struct usb_endpoint_descriptor *)*descriptors;
+ addr = ((ep->bEndpointAddress & 0x80) >> 3)
+ | (ep->bEndpointAddress & 0x0f);
+ set_bit(addr, f->endpoints);
+ }
+}
+
static int set_config(struct usb_composite_dev *cdev,
const struct usb_ctrlrequest *ctrl, unsigned number)
{
@@ -420,7 +444,6 @@ static int set_config(struct usb_composite_dev *cdev,
/* Initialize all interfaces by setting them to altsetting zero. */
for (tmp = 0; tmp < MAX_CONFIG_INTERFACES; tmp++) {
struct usb_function *f = c->interface[tmp];
- struct usb_descriptor_header **descriptors;
if (!f)
break;
@@ -431,23 +454,7 @@ static int set_config(struct usb_composite_dev *cdev,
* function's setup callback instead of the current
* configuration's setup callback.
*/
- if (gadget->speed == USB_SPEED_HIGH)
- descriptors = f->hs_descriptors;
- else
- descriptors = f->descriptors;
-
- for (; *descriptors; ++descriptors) {
- struct usb_endpoint_descriptor *ep;
- int addr;
-
- if ((*descriptors)->bDescriptorType != USB_DT_ENDPOINT)
- continue;
-
- ep = (struct usb_endpoint_descriptor *)*descriptors;
- addr = ((ep->bEndpointAddress & 0x80) >> 3)
- | (ep->bEndpointAddress & 0x0f);
- set_bit(addr, f->endpoints);
- }
+ record_endpoints(f, gadget->speed);
result = f->set_alt(f, tmp, 0);
if (result < 0) {
--
Best regards, _ _
| Humble Liege of Serenely Enlightened Majesty of o' \,=./ `o
| Computer Science, Michał "mina86" Nazarewicz (o o)
+----[mina86*mina86.com]---[mina86*jabber.org]----ooO--(_)--Ooo--
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
