lkml.org 
[lkml]   [2019]   [Sep]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.14 32/57] USB: cdc-wdm: fix race between write and disconnect due to flag abuse
    Date
    From: Oliver Neukum <oneukum@suse.com>

    commit 1426bd2c9f7e3126e2678e7469dca9fd9fc6dd3e upstream.

    In case of a disconnect an ongoing flush() has to be made fail.
    Nevertheless we cannot be sure that any pending URB has already
    finished, so although they will never succeed, they still must
    not be touched.
    The clean solution for this is to check for WDM_IN_USE
    and WDM_DISCONNECTED in flush(). There is no point in ever
    clearing WDM_IN_USE, as no further writes make sense.

    The issue is as old as the driver.

    Fixes: afba937e540c9 ("USB: CDC WDM driver")
    Reported-by: syzbot+d232cca6ec42c2edb3fc@syzkaller.appspotmail.com
    Signed-off-by: Oliver Neukum <oneukum@suse.com>
    Cc: stable <stable@vger.kernel.org>
    Link: https://lore.kernel.org/r/20190827103436.21143-1-oneukum@suse.com
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    drivers/usb/class/cdc-wdm.c | 16 ++++++++++++----
    1 file changed, 12 insertions(+), 4 deletions(-)

    --- a/drivers/usb/class/cdc-wdm.c
    +++ b/drivers/usb/class/cdc-wdm.c
    @@ -584,10 +584,20 @@ static int wdm_flush(struct file *file,
    {
    struct wdm_device *desc = file->private_data;

    - wait_event(desc->wait, !test_bit(WDM_IN_USE, &desc->flags));
    + wait_event(desc->wait,
    + /*
    + * needs both flags. We cannot do with one
    + * because resetting it would cause a race
    + * with write() yet we need to signal
    + * a disconnect
    + */
    + !test_bit(WDM_IN_USE, &desc->flags) ||
    + test_bit(WDM_DISCONNECTING, &desc->flags));

    /* cannot dereference desc->intf if WDM_DISCONNECTING */
    - if (desc->werr < 0 && !test_bit(WDM_DISCONNECTING, &desc->flags))
    + if (test_bit(WDM_DISCONNECTING, &desc->flags))
    + return -ENODEV;
    + if (desc->werr < 0)
    dev_err(&desc->intf->dev, "Error in flush path: %d\n",
    desc->werr);

    @@ -955,8 +965,6 @@ static void wdm_disconnect(struct usb_in
    spin_lock_irqsave(&desc->iuspin, flags);
    set_bit(WDM_DISCONNECTING, &desc->flags);
    set_bit(WDM_READ, &desc->flags);
    - /* to terminate pending flushes */
    - clear_bit(WDM_IN_USE, &desc->flags);
    spin_unlock_irqrestore(&desc->iuspin, flags);
    wake_up_all(&desc->wait);
    mutex_lock(&desc->rlock);

    \
     
     \ /
      Last update: 2019-09-04 20:24    [W:4.031 / U:0.120 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site