linux-kernel - Re: [PATCH 1/4 v3] usb: dwc2: Avoid sleeping while holding hsotg->lock

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: <CALAqxLUzOOjcBFFaAsCjXCfATOETGbXjmrhgoDRnecs5NjrEWg@mail.gmail.com>
Date:   Mon, 16 Jan 2017 15:28:33 -0800
From:   John Stultz <john.stultz@...aro.org>
To:     John Youn <John.Youn@...opsys.com>
Cc:     Felipe Balbi <felipe.balbi@...ux.intel.com>,
        lkml <linux-kernel@...r.kernel.org>,
        Wei Xu <xuwei5@...ilicon.com>,
        Guodong Xu <guodong.xu@...aro.org>,
        Amit Pundir <amit.pundir@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Douglas Anderson <dianders@...omium.org>,
        Chen Yu <chenyu56@...wei.com>,
        Vardan Mikayelyan <Vardan.Mikayelyan@...opsys.com>,
        Kishon Vijay Abraham I <kishon@...com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linux USB List <linux-usb@...r.kernel.org>
Subject: Re: [PATCH 1/4 v3] usb: dwc2: Avoid sleeping while holding hsotg->lock

On Mon, Jan 16, 2017 at 12:57 PM, John Youn <John.Youn@...opsys.com> wrote:
>
>
>> On Jan 16, 2017, at 12:37 PM, John Stultz <john.stultz@...aro.org> wrote:
>>
>> On Mon, Jan 16, 2017 at 2:36 AM, Felipe Balbi
>> <felipe.balbi@...ux.intel.com> wrote:
>>>
>>> Hi,
>>>
>>> John Stultz <john.stultz@...aro.org> writes:
>>>> Basically when plugging in various cables in different orders, I'm
>>>> occasionally seeing the following BUG splat:
>>>>
>>>> [   86.215403] BUG: scheduling while atomic: kworker/u16:2/53/0x00000002
>>>> [   86.219164] usb 1-1: USB disconnect, device number 9
>>>> [   86.226845] Preemption disabled at:[   86.230218]
>>>> [<ffffff8008673558>] dwc2_conn_id_status_change+0x120/0x250
>>>> [   86.236894] CPU: 0 PID: 53 Comm: kworker/u16:2 Tainted: G        W
>>>>     4.9.0-rc8-00051-gd5a7979-dirty #1702
>>>> [   86.246836] Hardware name: HiKey Development Board (DT)
>>>> [   86.252100] Workqueue: dwc2 dwc2_conn_id_status_change
>>>> [   86.257279] Call trace:
>>>> [   86.259771] [<ffffff8008087c28>] dump_backtrace+0x0/0x1a0
>>>> [   86.265210] [<ffffff8008087ddc>] show_stack+0x14/0x20
>>>> [   86.270308] [<ffffff80084343f0>] dump_stack+0x90/0xb0
>>>> [   86.275401] [<ffffff80080d8d94>] __schedule_bug+0x6c/0xb8
>>>> [   86.280841] [<ffffff8008a07220>] __schedule+0x4f8/0x5b0
>>>> [   86.286099] [<ffffff8008a073e8>] schedule+0x38/0xa0
>>>> [   86.291017] [<ffffff8008a0a6cc>] schedule_hrtimeout_range_clock+0x8c/0xf0
>>>> [   86.297846] [<ffffff8008a0a740>] schedule_hrtimeout_range+0x10/0x18
>>>> [   86.304150] [<ffffff8008a0a4a0>] usleep_range+0x50/0x58
>>>> [   86.309418] [<ffffff800866d8dc>] dwc2_wait_for_mode.isra.4+0x54/0xd0
>>>> [   86.315815] [<ffffff800866f058>] dwc2_core_reset+0xe0/0x168
>>>> [   86.321431] [<ffffff800867e364>] dwc2_hsotg_core_init_disconnected+0x2c/0x310
>>>> [   86.328602] [<ffffff8008673568>] dwc2_conn_id_status_change+0x130/0x250
>>>> [   86.335254] [<ffffff80080ccd48>] process_one_work+0x118/0x370
>>>> [   86.341035] [<ffffff80080ccfe8>] worker_thread+0x48/0x498
>>>> [   86.346473] [<ffffff80080d2eb0>] kthread+0xd0/0xe8
>>>> [   86.351299] [<ffffff8008082e80>] ret_from_fork+0x10/0x50
>>>>
>>>> This seems to be caused by the dwc2_wait_for_mode() calling
>>>> usleep_range() while the hstog->lock spinlock is held, since
>>>> we take that before calling dwc2_hsotg_core_init_disconnected().
>>>>
>>>> This patch avoids the issue by adding an extra argument to
>>>> dwc2_core_reset(), as suggested by John Youn, which allows us to
>>>> skip the waiting, which should be unnecessary when calling from
>>>> dwc2_hsotg_core_init_disconnected().
>>>>
>>>> Cc: Wei Xu <xuwei5@...ilicon.com>
>>>> Cc: Guodong Xu <guodong.xu@...aro.org>
>>>> Cc: Amit Pundir <amit.pundir@...aro.org>
>>>> Cc: Rob Herring <robh+dt@...nel.org>
>>>> Cc: John Youn <johnyoun@...opsys.com>
>>>> Cc: Douglas Anderson <dianders@...omium.org>
>>>> Cc: Chen Yu <chenyu56@...wei.com>
>>>> Cc: Vardan Mikayelyan <mvardan@...opsys.com>
>>>> Cc: Kishon Vijay Abraham I <kishon@...com>
>>>> Cc: Felipe Balbi <felipe.balbi@...ux.intel.com>
>>>> Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
>>>> Cc: linux-usb@...r.kernel.org
>>>> Signed-off-by: John Stultz <john.stultz@...aro.org>
>>>> ---
>>>
>>> doesn't apply to my testing/next. Please rebase
>>
>> So these were rebased onto JohnY's tree here:
>>  https://urldefense.proofpoint.com/v2/url?u=https-3A__github.com_synopsys-2Dusb_linux.git&d=DwIBaQ&c=DPL6_X_6JkXFx7AXWqB0tg&r=U3o8uKoKhWme5_V9D-eeCkB11BFwt4KvWztBgdE9ZpA&m=eBJTBuJyU21iKJvHdy5FxqtxsVARo0iIqJGxMHrlbyQ&s=VIiAT32aG7s04G5NoOOthNdm2JX0eWjJpg62neY_-KI&e=  next
>>
>> And apparently have been merged there. I suspect he's going to submit
>> his entire tree there to you?
>>
>> JohnY: Is this right?
>
>
> Yeah I'll get these issues sorted out with Felipe. Which may mean resubmitting everything the the proper order.

Ok. Thanks for sorting this out. Please let me know if there's
anything else you need from me!

thanks again!
-john