lkml.org 
[lkml]   [2018]   [Mar]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.14 097/101] usb: xhci: Fix potential memory leak in xhci_disable_slot()
    Date
    4.14-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Lu Baolu <baolu.lu@linux.intel.com>

    commit cd3f1790b006d91786728c20a01da21ee277aff1 upstream.

    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>

    ---
    drivers/usb/host/xhci-hub.c | 2 +-
    drivers/usb/host/xhci.c | 30 +++++++++---------------------
    drivers/usb/host/xhci.h | 3 +--
    3 files changed, 11 insertions(+), 24 deletions(-)

    --- a/drivers/usb/host/xhci-hub.c
    +++ b/drivers/usb/host/xhci-hub.c
    @@ -637,7 +637,7 @@ static int xhci_enter_test_mode(struct x
    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);
    --- a/drivers/usb/host/xhci.c
    +++ b/drivers/usb/host/xhci.c
    @@ -3523,11 +3523,6 @@ static void xhci_free_dev(struct usb_hcd
    struct xhci_virt_device *virt_dev;
    struct xhci_slot_ctx *slot_ctx;
    int i, ret;
    - struct xhci_command *command;
    -
    - command = xhci_alloc_command(xhci, false, false, GFP_KERNEL);
    - if (!command)
    - return;

    #ifndef CONFIG_USB_DEFAULT_PERSIST
    /*
    @@ -3543,10 +3538,8 @@ static void xhci_free_dev(struct usb_hcd
    /* 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);
    @@ -3558,22 +3551,21 @@ static void xhci_free_dev(struct usb_hcd
    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;

    @@ -3591,7 +3583,7 @@ int xhci_disable_slot(struct xhci_hcd *x
    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);
    @@ -3666,6 +3658,8 @@ int xhci_alloc_dev(struct usb_hcd *hcd,
    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);
    @@ -3701,18 +3695,12 @@ int xhci_alloc_dev(struct usb_hcd *hcd,
    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);
    }

    /*
    --- a/drivers/usb/host/xhci.h
    +++ b/drivers/usb/host/xhci.h
    @@ -2014,8 +2014,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);

    \
     
     \ /
      Last update: 2018-03-27 19:18    [W:4.281 / U:0.700 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site