linux-kernel - Re: [PATCH v2] usbip: vhci_hcd: fix calling usb_hcd_giveback

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAAeHK+zb=ieXi1ug_+OskcOLaWPAq2HSfxkwEmd=d1P0DfJx-w@mail.gmail.com>
Date:   Wed, 7 Oct 2020 01:16:01 +0200
From:   Andrey Konovalov <andreyknvl@...gle.com>
To:     Shuah Khan <skhan@...uxfoundation.org>
Cc:     Valentina Manea <valentina.manea.m@...il.com>,
        Shuah Khan <shuah@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Alan Stern <stern@...land.harvard.edu>,
        USB list <linux-usb@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] usbip: vhci_hcd: fix calling usb_hcd_giveback_urb()
 with irqs enabled

On Wed, Oct 7, 2020 at 12:39 AM Shuah Khan <skhan@...uxfoundation.org> wrote:
>
> kcov testing uncovered call to usb_hcd_giveback_urb() without disabling
> interrupts.
>
> Link: https://lore.kernel.org/linux-usb/CAAeHK+wb4k-LGTjK9F5YbJNviF_+yU+wE_=Vpo9Rn7KFN8vG6Q@mail.gmail.com/
>
> usb_hcd_giveback_urb() is called from vhci's urb_enqueue, when it
> determines it doesn't need to xmit the urb and can give it back.
> This path runs in task context.
>
> Disable irqs around usb_hcd_giveback_urb() call.
>
> Reported-by: Andrey Konovalov <andreyknvl@...gle.com>
> Suggested-by: Alan Stern <stern@...land.harvard.edu>
> Signed-off-by: Shuah Khan <skhan@...uxfoundation.org>

Acked-by: Andrey Konovalov <andreyknvl@...gle.com>

Thank you!

> ---
>
> Changes in v2: Changelog updated with correct information.
>  drivers/usb/usbip/vhci_hcd.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c
> index 1b598db5d8b9..66cde5e5f796 100644
> --- a/drivers/usb/usbip/vhci_hcd.c
> +++ b/drivers/usb/usbip/vhci_hcd.c
> @@ -797,8 +797,14 @@ static int vhci_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, gfp_t mem_flag
>         usb_hcd_unlink_urb_from_ep(hcd, urb);
>  no_need_unlink:
>         spin_unlock_irqrestore(&vhci->lock, flags);
> -       if (!ret)
> +       if (!ret) {
> +               /* usb_hcd_giveback_urb() should be called with
> +                * irqs disabled
> +                */
> +               local_irq_disable();
>                 usb_hcd_giveback_urb(hcd, urb, urb->status);
> +               local_irq_enable();
> +       }
>         return ret;
>  }
>
> --
> 2.25.1
>