Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: Is usb_hcd_giveback_urb() allowed in task context?
To:     Alan Stern <stern@rowland.harvard.edu>,
        Andrey Konovalov <andreyknvl@google.com>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Valentina Manea <valentina.manea.m@gmail.com>,
        Shuah Khan <shuah@kernel.org>,
        USB list <linux-usb@vger.kernel.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Dmitry Vyukov <dvyukov@google.com>,
        Nazime Hande Harputluoglu <handeharputlu@google.com>,
        syzkaller <syzkaller@googlegroups.com>,
        Shuah Khan <skhan@linuxfoundation.org>
References: <CAAeHK+wb4k-LGTjK9F5YbJNviF_+yU+wE_=Vpo9Rn7KFN8vG6Q@mail.gmail.com>
 <20201005151857.GA2309511@kroah.com>
 <CAAeHK+zes2Y00+EJ6SVtOHj8YCADw5WSXUEFHWCRgxi=H42+4w@mail.gmail.com>
 <20201005152540.GG376584@rowland.harvard.edu>
From:   Shuah Khan <skhan@linuxfoundation.org>
Message-ID: <65b4ff62-f9c8-b9cf-50bb-c9b08cce7230@linuxfoundation.org>
Date:   Mon, 5 Oct 2020 17:38:22 -0600
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101
 Thunderbird/68.10.0
MIME-Version: 1.0
In-Reply-To: <20201005152540.GG376584@rowland.harvard.edu>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 10/5/20 9:25 AM, Alan Stern wrote:
> On Mon, Oct 05, 2020 at 05:21:30PM +0200, Andrey Konovalov wrote:
>> On Mon, Oct 5, 2020 at 5:18 PM Greg Kroah-Hartman
>> <gregkh@linuxfoundation.org> wrote:
>>>
>>> On Mon, Oct 05, 2020 at 05:08:11PM +0200, Andrey Konovalov wrote:
>>>> Dear USB and USB/IP maintainers,
>>>>
>>>> While fuzzing the USB/IP stack with syzkaller we've stumbled upon an issue.
>>>>
>>>> Currently kcov (the subsystem that is used for coverage collection)
>>>> USB-related callbacks assume that usb_hcd_giveback_urb() can only be
>>>> called from interrupt context, as indicated by the comment before the
>>>> function definition. In the USB/IP code, however, it's called from the
>>>> task context (see the stack trace below).
>>>>
>>>> Is this something that is allowed and we need to fix kcov? Or is this
>>>> a bug in USB/IP?
>>>
>>> It's a bug in kcov, and is not true as you have found out :)
>>
>> OK, I see, I'll work on a fix, thanks!
>>
>> Should I also update the comment above usb_hcd_giveback_urb() to
>> mention that it can be called in_task()? Or is this redundant and is
>> assumed in general?
> 
> No, no -- it won't work right if it's called in process context.  Not
> only do the spinlock calls leave the interrupt flag unchanged, also the
> driver callback routines may expect to be invoked with interrupts
> disabled.  (We have tried to fix this, but I'm not at all certain that
> all the cases have been updated.)
> 

In the case of vhci case, 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.

Do you have any recommendation on how this case can be handled?

thanks,
-- Shuah