usb: xhci: Fix potential memory leak in xhci_disable_slot()

xhci_disable_slot() allows the invoker to pass a command pointer
as paramenter. Otherwise, it will allocate one. This will cause
memory leak when a command structure was allocated inside of this
function while queuing command trb fails. Another problem comes up
when the invoker passed a command pointer, but xhci_disable_slot()
frees it when it detects a dead host.

This patch fixes these two problems by removing the command parameter
from xhci_disable_slot().

Fixes: f9e609b82479 ("usb: xhci: Add helper function xhci_disable_slot().")
Cc: Guoqing Zhang <guoqing.zhang@intel.com>
Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/usb/host/xhci-hub.c b/drivers/usb/host/xhci-hub.c
index f0ae9df7cbef0b923a6f2579aa028105da19509a..e35903dc277551e0fd8241e14f59821c9c392407 100644 (file)
--- a/drivers/usb/host/xhci-hub.c
+++ b/drivers/usb/host/xhci-hub.c
@@ -615,7 +615,7 @@ static int xhci_enter_test_mode(struct xhci_hcd *xhci,
                if (!xhci->devs[i])
                        continue;
 
-               retval = xhci_disable_slot(xhci, NULL, i);
+               retval = xhci_disable_slot(xhci, i);
                if (retval)
                        xhci_err(xhci, "Failed to disable slot %d, %d. Enter test mode anyway\n",
                                 i, retval);

diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c
index 5cb7c5cd4af5c38728fc93d68ca7fc20f3a90a9b..d9dabb7046a8c3d48e8ecab430d361deece6d9ae 100644 (file)
--- a/drivers/usb/host/xhci.c
+++ b/drivers/usb/host/xhci.c
@@ -3531,14 +3531,9 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
        struct xhci_virt_device *virt_dev;
        struct xhci_slot_ctx *slot_ctx;
        int i, ret;
-       struct xhci_command *command;
 
        xhci_debugfs_remove_slot(xhci, udev->slot_id);
 
-       command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);
-       if (!command)
-               return;
-
 #ifndef CONFIG_USB_DEFAULT_PERSIST
        /*
         * We called pm_runtime_get_noresume when the device was attached.
@@ -3553,10 +3548,8 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
        /* If the host is halted due to driver unload, we still need to free the
         * device.
         */
-       if (ret <= 0 && ret != -ENODEV) {
-               kfree(command);
+       if (ret <= 0 && ret != -ENODEV)
                return;
-       }
 
        virt_dev = xhci->devs[udev->slot_id];
        slot_ctx = xhci_get_slot_ctx(xhci, virt_dev->out_ctx);
@@ -3568,22 +3561,21 @@ static void xhci_free_dev(struct usb_hcd *hcd, struct usb_device *udev)
                del_timer_sync(&virt_dev->eps[i].stop_cmd_timer);
        }
 
-       xhci_disable_slot(xhci, command, udev->slot_id);
+       xhci_disable_slot(xhci, udev->slot_id);
        /*
         * Event command completion handler will free any data structures
         * associated with the slot.  XXX Can free sleep?
         */
 }
 
-int xhci_disable_slot(struct xhci_hcd *xhci, struct xhci_command *command,
-                       u32 slot_id)
+int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id)
 {
+       struct xhci_command *command;
        unsigned long flags;
        u32 state;
        int ret = 0;
 
-       if (!command)
-               command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);
+       command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);
        if (!command)
                return -ENOMEM;
 
@@ -3602,7 +3594,7 @@ int xhci_disable_slot(struct xhci_hcd *xhci, struct xhci_command *command,
                                slot_id);
        if (ret) {
                spin_unlock_irqrestore(&xhci->lock, flags);
-               xhci_dbg(xhci, "FIXME: allocate a command ring segment\n");
+               kfree(command);
                return ret;
        }
        xhci_ring_cmd_db(xhci);
@@ -3677,6 +3669,8 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
                return 0;
        }
 
+       xhci_free_command(xhci, command);
+
        if ((xhci->quirks & XHCI_EP_LIMIT_QUIRK)) {
                spin_lock_irqsave(&xhci->lock, flags);
                ret = xhci_reserve_host_control_ep_resources(xhci);
@@ -3714,18 +3708,12 @@ int xhci_alloc_dev(struct usb_hcd *hcd, struct usb_device *udev)
                pm_runtime_get_noresume(hcd->self.controller);
 #endif
 
-
-       xhci_free_command(xhci, command);
        /* Is this a LS or FS device under a HS hub? */
        /* Hub or peripherial? */
        return 1;
 
 disable_slot:
-       /* Disable slot, if we can do it without mem alloc */
-       kfree(command->completion);
-       command->completion = NULL;
-       command->status = 0;
-       return xhci_disable_slot(xhci, command, udev->slot_id);
+       return xhci_disable_slot(xhci, udev->slot_id);
 }
 
 /*

diff --git a/drivers/usb/host/xhci.h b/drivers/usb/host/xhci.h
index ccf0ca3ed71e728092b2d6dd3150e543cd191ce7..7379309f4d1f87b2f44a6c5e9426f1c7dc3c539f 100644 (file)
--- a/drivers/usb/host/xhci.h
+++ b/drivers/usb/host/xhci.h
@@ -2019,8 +2019,7 @@ int xhci_run(struct usb_hcd *hcd);
 int xhci_gen_setup(struct usb_hcd *hcd, xhci_get_quirks_t get_quirks);
 void xhci_init_driver(struct hc_driver *drv,
                      const struct xhci_driver_overrides *over);
-int xhci_disable_slot(struct xhci_hcd *xhci,
-                       struct xhci_command *command, u32 slot_id);
+int xhci_disable_slot(struct xhci_hcd *xhci, u32 slot_id);
 
 int xhci_suspend(struct xhci_hcd *xhci, bool do_wakeup);
 int xhci_resume(struct xhci_hcd *xhci, bool hibernated);