lkml.org 
[lkml]   [2019]   [Jul]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.9 37/54] carl9170: fix misuse of device driver API
    Date
    From: Christian Lamparter <chunkeey@gmail.com>

    commit feb09b2933275a70917a869989ea2823e7356be8 upstream.

    This patch follows Alan Stern's recent patch:
    "p54: Fix race between disconnect and firmware loading"

    that overhauled carl9170 buggy firmware loading and driver
    unbinding procedures.

    Since the carl9170 code was adapted from p54 it uses the
    same functions and is likely to have the same problem, but
    it's just that the syzbot hasn't reproduce them (yet).

    a summary from the changes (copied from the p54 patch):
    * Call usb_driver_release_interface() rather than
    device_release_driver().

    * Lock udev (the interface's parent) before unbinding the
    driver instead of locking udev->parent.

    * During the firmware loading process, take a reference
    to the USB interface instead of the USB device.

    * Don't take an unnecessary reference to the device during
    probe (and then don't drop it during disconnect).

    and

    * Make sure to prevent use-after-free bugs by explicitly
    setting the driver context to NULL after signaling the
    completion.

    Cc: <stable@vger.kernel.org>
    Cc: Alan Stern <stern@rowland.harvard.edu>
    Signed-off-by: Christian Lamparter <chunkeey@gmail.com>
    Acked-by: Alan Stern <stern@rowland.harvard.edu>
    Signed-off-by: Kalle Valo <kvalo@codeaurora.org>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    drivers/net/wireless/ath/carl9170/usb.c | 39 +++++++++++++-------------------
    1 file changed, 17 insertions(+), 22 deletions(-)

    --- a/drivers/net/wireless/ath/carl9170/usb.c
    +++ b/drivers/net/wireless/ath/carl9170/usb.c
    @@ -128,6 +128,8 @@ static struct usb_device_id carl9170_usb
    };
    MODULE_DEVICE_TABLE(usb, carl9170_usb_ids);

    +static struct usb_driver carl9170_driver;
    +
    static void carl9170_usb_submit_data_urb(struct ar9170 *ar)
    {
    struct urb *urb;
    @@ -966,32 +968,28 @@ err_out:

    static void carl9170_usb_firmware_failed(struct ar9170 *ar)
    {
    - struct device *parent = ar->udev->dev.parent;
    - struct usb_device *udev;
    -
    - /*
    - * Store a copy of the usb_device pointer locally.
    - * This is because device_release_driver initiates
    - * carl9170_usb_disconnect, which in turn frees our
    - * driver context (ar).
    + /* Store a copies of the usb_interface and usb_device pointer locally.
    + * This is because release_driver initiates carl9170_usb_disconnect,
    + * which in turn frees our driver context (ar).
    */
    - udev = ar->udev;
    + struct usb_interface *intf = ar->intf;
    + struct usb_device *udev = ar->udev;

    complete(&ar->fw_load_wait);
    + /* at this point 'ar' could be already freed. Don't use it anymore */
    + ar = NULL;

    /* unbind anything failed */
    - if (parent)
    - device_lock(parent);
    -
    - device_release_driver(&udev->dev);
    - if (parent)
    - device_unlock(parent);
    + usb_lock_device(udev);
    + usb_driver_release_interface(&carl9170_driver, intf);
    + usb_unlock_device(udev);

    - usb_put_dev(udev);
    + usb_put_intf(intf);
    }

    static void carl9170_usb_firmware_finish(struct ar9170 *ar)
    {
    + struct usb_interface *intf = ar->intf;
    int err;

    err = carl9170_parse_firmware(ar);
    @@ -1009,7 +1007,7 @@ static void carl9170_usb_firmware_finish
    goto err_unrx;

    complete(&ar->fw_load_wait);
    - usb_put_dev(ar->udev);
    + usb_put_intf(intf);
    return;

    err_unrx:
    @@ -1052,7 +1050,6 @@ static int carl9170_usb_probe(struct usb
    return PTR_ERR(ar);

    udev = interface_to_usbdev(intf);
    - usb_get_dev(udev);
    ar->udev = udev;
    ar->intf = intf;
    ar->features = id->driver_info;
    @@ -1094,15 +1091,14 @@ static int carl9170_usb_probe(struct usb
    atomic_set(&ar->rx_anch_urbs, 0);
    atomic_set(&ar->rx_pool_urbs, 0);

    - usb_get_dev(ar->udev);
    + usb_get_intf(intf);

    carl9170_set_state(ar, CARL9170_STOPPED);

    err = request_firmware_nowait(THIS_MODULE, 1, CARL9170FW_NAME,
    &ar->udev->dev, GFP_KERNEL, ar, carl9170_usb_firmware_step2);
    if (err) {
    - usb_put_dev(udev);
    - usb_put_dev(udev);
    + usb_put_intf(intf);
    carl9170_free(ar);
    }
    return err;
    @@ -1131,7 +1127,6 @@ static void carl9170_usb_disconnect(stru

    carl9170_release_firmware(ar);
    carl9170_free(ar);
    - usb_put_dev(udev);
    }

    #ifdef CONFIG_PM

    \
     
     \ /
      Last update: 2019-07-18 05:18    [W:4.549 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site