linux-kernel - Re: [PATCH v2 1/2] usb: ulpi: defer ulpi_register on ulpi_read

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]

Message-ID: <b8fb2ead-6177-ea22-4d5c-a10a994eda2d@gmail.com>
Date:   Mon, 14 Nov 2022 22:55:38 +0100
From:   Ferry Toth <fntoth@...il.com>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Ferry Toth <ftoth@...londelft.nl>
Cc:     linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org,
        Heikki Krogerus <heikki.krogerus@...ux.intel.com>,
        Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
        Sean Anderson <sean.anderson@...o.com>,
        Liu Shixin <liushixin2@...wei.com>,
        Andrey Smirnov <andrew.smirnov@...il.com>,
        Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        Stephen Boyd <swboyd@...omium.org>
Subject: Re: [PATCH v2 1/2] usb: ulpi: defer ulpi_register on ulpi_read_id
 timeout

Op 11-11-2022 om 07:08 schreef Greg Kroah-Hartman:
> On Thu, Nov 10, 2022 at 10:11:31PM +0100, Ferry Toth wrote:
>> Since commit 0f010171
>> Dual Role support on Intel Merrifield platform broke due to rearranging
>> the call to dwc3_get_extcon().
> 
> Please see the kernel documentation for how to refer to commits.  This
> should be written as:
> 
> 	Since commit 0f0101719138 ("usb: dwc3: Don't switch OTG -> peripheral if
> 	extcon is present"), Dual role....

Thanks I'll fix that in v3.

>> It appears to be caused by ulpi_read_id() on the first test write failing
>> with -ETIMEDOUT. Currently ulpi_read_id() expects to discover the phy via
>> DT when the test write fails and returns 0 in that case even if DT does not
>> provide the phy. As a result usb probe completes without phy.
>>
>> Signed-off-by: Ferry Toth <ftoth@...londelft.nl>
> 
> What commit does this fix?

It's complicated, not sure how to explain this clearly:
ef6a7bcfb01c ("usb: ulpi: Support device discovery via DT") started to 
hide -ETIMEDOUT by returning 0. That problem was hidden due to another
problem causing dwc3 to be deferred. But not properly, causing an 
infinite probe loop. This was fixed for quite some time by an out of 
tree patch. Now 0f0101719138 ("usb: dwc3: Don't switch OTG -> peripheral 
if extcon is present") accidentally fixes the probe loop, makes the out 
tree patch obsolete, but exposes the initial problem.

In short this patch fixes ef6a7bcfb01c ("usb: ulpi: Support device 
discovery via DT") by returning -ETIMEDOUT to its user, who should 
handle it appropriately. In case of dwc3 probe it sets -EPROBE_DEFER and 
bails out.

I'll add the short fixes: in v3.

> Should this also get a cc: stable?

I will add.

> thanks,
> 
> greg k-h