Skip to content

Commit

Permalink
usb: gadget: composite: don't call driver's unbind() if bind() failed
Browse files Browse the repository at this point in the history
Lets assume nokia_bind() starts with "return -EINVAL". After loading the
gadget we end up with:

|udc dummy_udc.0: registering UDC driver [g_nokia]
|BUG: unable to handle kernel NULL pointer dereference at 00000040
|IP: [<c11f9555>] __list_add+0x25/0xf0
|Call Trace:
| [<c12d4e21>] rollback_registered+0x21/0x40
| [<c12d513f>] unregister_netdevice_queue+0x4f/0xa0
| [<c12d5259>] unregister_netdev+0x19/0x30
| [<f81335b2>] gphonet_cleanup+0x32/0x50 [g_nokia]
| [<f8133f1c>] nokia_unbind+0x1c/0x2a [g_nokia]
| [<f802509f>] __composite_unbind.constprop.10+0x4f/0xb0 [libcomposite]
| [<f80255be>] composite_bind+0x1ae/0x230 [libcomposite]
| [<c129e576>] usb_gadget_probe_driver+0xc6/0x1b0
| [<f8024aba>] usb_composite_probe+0x7a/0xa0 [libcomposite]

That is crash from nokia_unbind() invoked via nokia_bind(). This crash
will look different we if make it until usb_string_ids_tab() before we
enter an error condition in the probe function.
nokia_bind_config() tries to clean up which is IMHO the right thing to
do. Leaving things as-is and hoping that its unbind() will clean it up
is kinda backwards. Especially since the bind function never succeeded so
it can't know how much it needs to clean up.
This fixes the behaviour by not calling the driver's unbind function if
its bind function failed.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Signed-off-by: Felipe Balbi <balbi@ti.com>
  • Loading branch information
Sebastian Andrzej Siewior authored and Felipe Balbi committed Jan 21, 2013
1 parent afd2e18 commit 779d516
Showing 1 changed file with 8 additions and 4 deletions.
12 changes: 8 additions & 4 deletions drivers/usb/gadget/composite.c
Original file line number Diff line number Diff line change
Expand Up @@ -1349,8 +1349,7 @@ static ssize_t composite_show_suspended(struct device *dev,

static DEVICE_ATTR(suspended, 0444, composite_show_suspended, NULL);

static void
composite_unbind(struct usb_gadget *gadget)
static void __composite_unbind(struct usb_gadget *gadget, bool unbind_driver)
{
struct usb_composite_dev *cdev = get_gadget_data(gadget);

Expand All @@ -1367,7 +1366,7 @@ composite_unbind(struct usb_gadget *gadget)
struct usb_configuration, list);
remove_config(cdev, c);
}
if (cdev->driver->unbind)
if (cdev->driver->unbind && unbind_driver)
cdev->driver->unbind(cdev);

if (cdev->req) {
Expand All @@ -1380,6 +1379,11 @@ composite_unbind(struct usb_gadget *gadget)
set_gadget_data(gadget, NULL);
}

static void composite_unbind(struct usb_gadget *gadget)
{
__composite_unbind(gadget, true);
}

static void update_unchanged_dev_desc(struct usb_device_descriptor *new,
const struct usb_device_descriptor *old)
{
Expand Down Expand Up @@ -1488,7 +1492,7 @@ static int composite_bind(struct usb_gadget *gadget,
return 0;

fail:
composite_unbind(gadget);
__composite_unbind(gadget, false);
return status;
}

Expand Down

0 comments on commit 779d516

Please sign in to comment.