ChangeSet 1.1722.97.46, 2004/06/07 16:55:17-07:00, [EMAIL PROTECTED]
[PATCH] USB: Fix resource leakage in the hub driver
The hub driver is very careless about returning resources when an error
occurs while installing a new device. This patch attempts to put some
order back into the situation. Details:
Since usb_new_device() allocates neither the device structure
nor the device address, it shouldn't release either one.
Because usb_new_device() no longer releases the device structure,
usb_register_root_hub() doesn't need to take an extra reference
to it.
Since the device address selection and TT setup code is used
only for new devices, not ones being reset, move that code from
hub_port_init() to hub_port_connect_change(). By the same token,
hub_port_init() doesn't have to release the device address or
the device structure.
Just to make things look better, move the failure code in
hub_port_init() to the end of the routine. And when disabling
endpoint 0, disable both the IN and OUT parts of the endpoint.
In hub_port_connect_change(), make all the failure paths
execute the same code so that resources are always released.
These resources comprise: the pointer from the parent to the
new child device, the HCD state for ep0, the device's address,
and the device structure itself -- in short, everything that's
set up before calling usb_new_device().
Signed-off-by: Alan Stern <[EMAIL PROTECTED]>
Signed-off-by: Greg Kroah-Hartman <[EMAIL PROTECTED]>
drivers/usb/core/hcd.c | 4 --
drivers/usb/core/hub.c | 95 +++++++++++++++++++++++--------------------------
2 files changed, 46 insertions(+), 53 deletions(-)
diff -Nru a/drivers/usb/core/hcd.c b/drivers/usb/core/hcd.c
--- a/drivers/usb/core/hcd.c Fri Jun 18 11:00:31 2004
+++ b/drivers/usb/core/hcd.c Fri Jun 18 11:00:31 2004
@@ -787,14 +787,12 @@
return (retval < 0) ? retval : -EMSGSIZE;
}
- (void) usb_get_dev (usb_dev);
down (&usb_dev->serialize);
retval = usb_new_device (usb_dev);
+ up (&usb_dev->serialize);
if (retval)
dev_err (parent_dev, "can't register root hub for %s, %d\n",
usb_dev->dev.bus_id, retval);
- up (&usb_dev->serialize);
- usb_put_dev (usb_dev);
return retval;
}
EXPORT_SYMBOL (usb_register_root_hub);
diff -Nru a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
--- a/drivers/usb/core/hub.c Fri Jun 18 11:00:31 2004
+++ b/drivers/usb/core/hub.c Fri Jun 18 11:00:31 2004
@@ -1055,8 +1055,6 @@
fail:
udev->state = USB_STATE_NOTATTACHED;
- release_address(udev);
- usb_put_dev(udev);
return err;
}
@@ -1334,33 +1332,9 @@
udev->epmaxpacketin [0] = i;
udev->epmaxpacketout[0] = i;
- /* set the address */
- if (udev->devnum <= 0) {
- choose_address(udev);
- if (udev->devnum <= 0)
- goto fail;
-
- /* Set up TT records, if needed */
- if (hdev->tt) {
- udev->tt = hdev->tt;
- udev->ttport = hdev->ttport;
- } else if (udev->speed != USB_SPEED_HIGH
- && hdev->speed == USB_SPEED_HIGH) {
- struct usb_hub *hub;
-
- hub = usb_get_intfdata (hdev->actconfig
- ->interface[0]);
- udev->tt = &hub->tt;
- udev->ttport = port + 1;
- }
-
- /* force the right log message (below) at low speed */
- oldspeed = USB_SPEED_UNKNOWN;
- }
-
dev_info (&udev->dev,
"%s %s speed USB device using address %d\n",
- (oldspeed == USB_SPEED_UNKNOWN) ? "new" : "reset",
+ (udev->config) ? "reset" : "new",
({ char *speed; switch (udev->speed) {
case USB_SPEED_LOW: speed = "low"; break;
case USB_SPEED_FULL: speed = "full"; break;
@@ -1389,12 +1363,7 @@
dev_err(&udev->dev,
"device not accepting address %d, error %d\n",
udev->devnum, retval);
- fail:
- hub_port_disable(hdev, port);
- release_address(udev);
- usb_put_dev(udev);
- up(&usb_address0_sem);
- return retval;
+ goto fail;
}
/* cope with hardware quirkiness:
@@ -1417,7 +1386,8 @@
if (udev->speed == USB_SPEED_FULL
&& (udev->epmaxpacketin [0]
!= udev->descriptor.bMaxPacketSize0)) {
- usb_disable_endpoint(udev, 0);
+ usb_disable_endpoint(udev, 0 + USB_DIR_IN);
+ usb_disable_endpoint(udev, 0 + USB_DIR_OUT);
usb_endpoint_running(udev, 0, 1);
usb_endpoint_running(udev, 0, 0);
udev->epmaxpacketin [0] = udev->descriptor.bMaxPacketSize0;
@@ -1435,9 +1405,11 @@
/* now dev is visible to other tasks */
hdev->children[port] = udev;
+ retval = 0;
+fail:
up(&usb_address0_sem);
- return 0;
+ return retval;
}
static void
@@ -1562,20 +1534,36 @@
goto done;
}
udev->state = USB_STATE_POWERED;
-
+
/* hub can tell if it's lowspeed already: D- pullup (not D+) */
if (portstatus & USB_PORT_STAT_LOW_SPEED)
udev->speed = USB_SPEED_LOW;
else
udev->speed = USB_SPEED_UNKNOWN;
- /* reset, set address, get descriptor, add to hub's children */
down (&udev->serialize);
+
+ /* set the address */
+ choose_address(udev);
+ if (udev->devnum <= 0) {
+ status = -ENOTCONN; /* Don't retry */
+ goto loop;
+ }
+
+ /* reset, get descriptor, add to hub's children */
status = hub_port_init(hdev, udev, port);
- if (status == -ENOTCONN)
- break;
if (status < 0)
- continue;
+ goto loop;
+
+ /* Set up TT records, if needed */
+ if (hdev->tt) {
+ udev->tt = hdev->tt;
+ udev->ttport = hdev->ttport;
+ } else if (udev->speed != USB_SPEED_HIGH
+ && hdev->speed == USB_SPEED_HIGH) {
+ udev->tt = &hub->tt;
+ udev->ttport = port + 1;
+ }
/* consecutive bus-powered hubs aren't reliable; they can
* violate the voltage drop budget. if the new child has
@@ -1591,7 +1579,7 @@
&devstat);
if (status < 0) {
dev_dbg(&udev->dev, "get status %d ?\n", status);
- continue;
+ goto loop;
}
cpu_to_le16s(&devstat);
if ((devstat & (1 << USB_DEVICE_SELF_POWERED)) == 0) {
@@ -1603,10 +1591,8 @@
INDICATOR_AMBER_BLINK;
schedule_work (&hub->leds);
}
- hdev->children[port] = NULL;
- usb_put_dev(udev);
- hub_port_disable(hdev, port);
- return;
+ status = -ENOTCONN; /* Don't retry */
+ goto loop;
}
}
@@ -1618,11 +1604,8 @@
/* Run it through the hoops (find a driver, etc) */
status = usb_new_device(udev);
- if (status != 0) {
- hdev->children[port] = NULL;
- continue;
- }
- up (&udev->serialize);
+ if (status)
+ goto loop;
status = hub_power_remaining(hub, hdev);
if (status)
@@ -1630,7 +1613,19 @@
"%dmA power budget left\n",
2 * status);
+ up (&udev->serialize);
return;
+
+loop:
+ hdev->children[port] = NULL;
+ hub_port_disable(hdev, port);
+ usb_disable_endpoint(udev, 0 + USB_DIR_IN);
+ usb_disable_endpoint(udev, 0 + USB_DIR_OUT);
+ release_address(udev);
+ up (&udev->serialize);
+ usb_put_dev(udev);
+ if (status == -ENOTCONN)
+ break;
}
done:
-------------------------------------------------------
This SF.Net email is sponsored by The 2004 JavaOne(SM) Conference
Learn from the experts at JavaOne(SM), Sun's Worldwide Java Developer
Conference, June 28 - July 1 at the Moscone Center in San Francisco, CA
REGISTER AND SAVE! http://java.sun.com/javaone/sf Priority Code NWMGYKND
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
