Hi Xenia,
thank you for the patch. I tried to reproduce the error with patched 3.10.9
kernel but it seems the kmemleak is indeed gone. Provided I get only these lines
logged which used to be followed by kmemleak findings I believe the original
fixed:
[15885.206032] usb 4-2.1: reset SuperSpeed USB device number 3 using xhci_hcd
[15885.225856] usb 4-2.1: Parent hub missing LPM exit latency info. Power management will be impacted.
[15885.228445] xhci_hcd 0000:0b:00.0: xHCI xhci_drop_endpoint called with disabled ep ffff8803ff81b1c8
[15885.228453] xhci_hcd 0000:0b:00.0: xHCI xhci_drop_endpoint called with disabled ep ffff8803ff81b208
[41990.166310] usb 4-2.1: reset SuperSpeed USB device number 9 using xhci_hcd
[41990.187276] usb 4-2.1: Parent hub missing LPM exit latency info. Power management will be impacted.
[41990.189285] xhci_hcd 0000:0b:00.0: xHCI xhci_drop_endpoint called with disabled ep ffff8803ca0381c8
[41990.189287] xhci_hcd 0000:0b:00.0: xHCI xhci_drop_endpoint called with disabled ep ffff8803ca038208
[65622.903882] usb 4-2.2: reset SuperSpeed USB device number 10 using xhci_hcd
[65622.927980] usb 4-2.2: Parent hub missing LPM exit latency info. Power management will be impacted.
[65622.929986] xhci_hcd 0000:0b:00.0: xHCI xhci_drop_endpoint called with disabled ep ffff8803ff81b130
[65622.929989] xhci_hcd 0000:0b:00.0: xHCI xhci_drop_endpoint called with disabled ep ffff8803ff81b170
Thank you,
Martin
Xenia Ragiadakou wrote:
> In usb_reset_and_verify_device(), hub_port_init() allocates a new bos
> descriptor to hold the value read by the device. The new bos descriptor
> has to be compared with the old one in order to figure out if device 's
> firmware has changed in which case the device has to be reenumerated.
> In the original code, none of the two descriptors was deallocated leading
> to memory leaks.
>
> This patch compares the old bos descriptor with the new one to detect change
> in firmware and releases the newly allocated bos descriptor to prevent memory
> leak.
>
> Signed-off-by: Xenia Ragiadakou <burzalodowa@xxxxxxxxx>
> ---
>
> Differences from version 2:
>
> - fix identation
> - initialize udev->bos to null so that check fails if bos is uninitialized
> - move bos deallocation inside 'done' and 're_enumerate' paths to ensure
> that the deallocation will be performed even if hub_port_init() fails
> - remove check (!udev->wusb && le16_to_cpu(udev->descriptor.bcdUSB) >= 0x0201)
> since the checks that follow suffice
>
> drivers/usb/core/hub.c | 23 +++++++++++++++++++++--
> 1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 175179e..2455001 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -4939,7 +4939,8 @@ void usb_hub_cleanup(void)
> } /* usb_hub_cleanup() */
>
> static int descriptors_changed(struct usb_device *udev,
> - struct usb_device_descriptor *old_device_descriptor)
> + struct usb_device_descriptor *old_device_descriptor,
> + struct usb_host_bos *old_bos)
> {
> int changed = 0;
> unsigned index;
> @@ -4953,6 +4954,16 @@ static int descriptors_changed(struct usb_device *udev,
> sizeof(*old_device_descriptor)) != 0)
> return 1;
>
> + if ((old_bos && !udev->bos) || (!old_bos && udev->bos))
> + return 1;
> + if (udev->bos) {
> + len = udev->bos->desc->wTotalLength;
> + if (len != old_bos->desc->wTotalLength)
> + return 1;
> + if (memcmp(udev->bos->desc, old_bos->desc, le16_to_cpu(len)))
> + return 1;
> + }
> +
> /* Since the idVendor, idProduct, and bcdDevice values in the
> * device descriptor haven't changed, we will assume the
> * Manufacturer and Product strings haven't changed either.
> @@ -5049,6 +5060,7 @@ static int usb_reset_and_verify_device(struct usb_device *udev)
> struct usb_hub *parent_hub;
> struct usb_hcd *hcd = bus_to_hcd(udev->bus);
> struct usb_device_descriptor descriptor = udev->descriptor;
> + struct usb_host_bos *bos;
> int i, ret = 0;
> int port1 = udev->portnum;
>
> @@ -5066,6 +5078,9 @@ static int usb_reset_and_verify_device(struct usb_device *udev)
> }
> parent_hub = usb_hub_to_struct_hub(parent_hdev);
>
> + bos = udev->bos;
> + udev->bos = NULL;
> +
> /* Disable LPM and LTM while we reset the device and reinstall the alt
> * settings. Device-initiated LPM settings, and system exit latency
> * settings are cleared when the device is reset, so we have to set
> @@ -5099,7 +5114,7 @@ static int usb_reset_and_verify_device(struct usb_device *udev)
> goto re_enumerate;
>
> /* Device might have changed firmware (DFU or similar) */
> - if (descriptors_changed(udev, &descriptor)) {
> + if (descriptors_changed(udev, &descriptor, bos)) {
> dev_info(&udev->dev, "device firmware changed\n");
> udev->descriptor = descriptor; /* for disconnect() calls */
> goto re_enumerate;
> @@ -5172,11 +5187,15 @@ done:
> /* Now that the alt settings are re-installed, enable LTM and LPM. */
> usb_unlocked_enable_lpm(udev);
> usb_enable_ltm(udev);
> + usb_release_bos_descriptor(udev);
> + udev->bos = bos;
> return 0;
>
> re_enumerate:
> /* LPM state doesn't matter when we're about to destroy the device. */
> hub_port_logical_disconnect(parent_hub, port1);
> + usb_release_bos_descriptor(udev);
> + udev->bos = bos;
> return -ENODEV;
> }
>
>
--
Martin Mokrejs, Ph.D.
Bioinformatics
Donovalska 1658
149 00 Prague
Czech Republic
http://www.iresite.org
http://www.iresite.org/~mmokrejs
--
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
