linux-kernel - Re: [PATCH] usb: dwc2: host: Fix remote wakeup when not in DWC2

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] [thread-next>] [day] [month] [year] [list]

Date:	Thu, 29 Oct 2015 16:45:10 -0700
From:	Doug Anderson <dianders@...omium.org>
To:	Heiko Stuebner <heiko@...ech.de>
Cc:	John Youn <John.Youn@...opsys.com>, Felipe Balbi <balbi@...com>,
	wulf <wulf@...k-chips.com>, lyz <lyz@...k-chips.com>,
	"open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
	Julius Werner <jwerner@...omium.org>,
	"Herrero, Gregory" <gregory.herrero@...el.com>,
	"Kaukab, Yousaf" <yousaf.kaukab@...el.com>, r.baldyga@...sung.com,
	Dinh Nguyen <dinguyen@...nsource.altera.com>,
	John Youn <johnyoun@...opsys.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] usb: dwc2: host: Fix remote wakeup when not in DWC2_L2

Hi,

On Thu, Oct 29, 2015 at 3:49 PM, Heiko Stuebner <heiko@...ech.de> wrote:
> Am Donnerstag, 22. Oktober 2015, 13:05:03 schrieb Douglas Anderson:
>> In commit 734643dfbdde ("usb: dwc2: host: add flag to reflect bus
>> state") we changed dwc2_port_suspend() not to set the lx_state
>> anymore (instead it sets the new bus_suspended variable).  This
>> introduced a bug where we would fail to detect device insertions if:
>>
>> 1. Plug empty hub into dwc2
>> 2. Plug USB flash drive into the empty hub.
>> 3. Wait a few seconds
>> 4. Unplug USB flash drive
>> 5. Less than 2 seconds after step 4, plug the USB flash drive in again.
>>
>> The dwc2_hcd_rem_wakeup() function should have been changed to look at
>> the new bus_suspended variable.
>>
>> Let's fix it.  Since commit b46146d59fda ("usb: dwc2: host: resume root
>> hub on remote wakeup") talks about needing the root hub resumed if the
>> bus was suspended, we'll include it in our test.
>>
>> It appears that the "port_l1_change" should only be set to 1 if we were
>> in DWC2_L1 (the driver currently never sets this), so we'll update the
>> former "else" case based on this test.
>>
>> Fixes: 734643dfbdde ("usb: dwc2: host: add flag to reflect bus state")
>> Signed-off-by: Douglas Anderson <dianders@...omium.org>
>
> I've talked with Doug a lot about that problem today and from reading
> this change and the referenced causing change, it looks correct
> and good to me, so

It does also appear that the steps in the commit message are not
sufficient to reproduce the problem.  Apparently something in the way
my system is configured adds a delay before the bus actually gets
suspended.  Presumably you could simulate the setup of my current
system with an msleep() at the start of _dwc2_hcd_suspend() (before
grabbing the spinlock).  I've stepped away from my test machine for
the day though, so I can't confirm that.

-Doug
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/