linux-kernel - RE: [PATCH v3] usb: hub: lack of clearing xHC resources

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:
 <PH7PR07MB953881F6F9DFA2DCFFFA2F97DD99A@PH7PR07MB9538.namprd07.prod.outlook.com>
Date: Thu, 22 May 2025 10:27:36 +0000
From: Pawel Laszczak <pawell@...ence.com>
To: Michał Pecio <michal.pecio@...il.com>
CC: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "stern@...land.harvard.edu" <stern@...land.harvard.edu>,
        "krzysztof.kozlowski@...aro.org" <krzysztof.kozlowski@...aro.org>,
        "christophe.jaillet@...adoo.fr" <christophe.jaillet@...adoo.fr>,
        "javier.carrasco@...fvision.net" <javier.carrasco@...fvision.net>,
        "make_ruc2021@....com" <make_ruc2021@....com>,
        "peter.chen@....com"
	<peter.chen@....com>,
        "linux-usb@...r.kernel.org"
	<linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>,
        Pawel Eichler <peichler@...ence.com>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH v3] usb: hub: lack of clearing xHC resources

>>
>>On Fri, 28 Feb 2025 07:50:25 +0000, Pawel Laszczak wrote:
>>> The xHC resources allocated for USB devices are not released in
>>> correct order after resuming in case when while suspend device was
>>> reconnected.
>>>
>>> This issue has been detected during the fallowing scenario:
>>> - connect hub HS to root port
>>> - connect LS/FS device to hub port
>>> - wait for enumeration to finish
>>> - force host to suspend
>>> - reconnect hub attached to root port
>>> - wake host
>>>
>>> For this scenario during enumeration of USB LS/FS device the Cadence
>>> xHC reports completion error code for xHC commands because the xHC
>>> resources used for devices has not been properly released.
>>> XHCI specification doesn't mention that device can be reset in any
>>> order so, we should not treat this issue as Cadence xHC controller
>>> bug. Similar as during disconnecting in this case the device
>>> resources should be cleared starting form the last usb device in tree
>>> toward the root hub. To fix this issue usbcore driver should call
>>> hcd->driver->reset_device for all USB devices connected to hub which
>>> was reconnected while suspending.
>>>
>>> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence
>>> USBSSP DRD Driver")
>>> cc: <stable@...r.kernel.org>
>>> Signed-off-by: Pawel Laszczak <pawell@...ence.com>
>>
>>Taking discussion about this patch out of bugzilla
>>https://urldefense.com/v3/__https://bugzilla.kernel.org/show_bug.cgi?id
>>=220
>>069*c42__;Iw!!EHscmS1ygiU1lA!FD7UdYLwKPptb8LI646boayHRFMR7zLGkto
>3
>>rhb0whLx1-CVUGaYVVgrG5Y6EyLj-QcTuuUHSpaZcVPaTTRM$
>>
>>As Mathias pointed out, this whole idea is an explicit spec violation,
>>because it puts multiple Device Slots into the Default state.
>>
>>(Which has nothing to do with actually resetting the devices, by the
>>way. USB core will still do it from the root towards the leaves. It
>>only means the xHC believes that they are reset when they are not.)
>>
>>
>>A reset-resume of a whole tree looks like a tricky case, because on one
>>hand a hub must resume (here: be reset) before its children in order to
>>reset them, but this apparently causes problems when some xHCs consider
>>the hub "in use" by the children (or its TT in use by their endpoints,
>>I suspect that's the case here and the reason why this patch helps).
>>
>>I have done some experimentation with reset-resuming from autosuspend,
>>either by causing Transaction Errors while resuming (full speed only)
>>or simulating usb_get_std_status() error in finish_port_resume().
>>
>>Either way, I noticed that the whole device tree ends up logically
>>disconnected and reconnected during reset-resume, so perhaps it would
>>be acceptable to disable all xHC Device Slots (leaf to root) before
>>resetting everything and re-enable Slots (root to leaf) one by one?
>

What about such fix:

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index cfb3abafeacd..46e640679eac 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -6093,8 +6093,8 @@ static void hub_hc_release_resources(struct usb_device *udev)
                if (hub->ports[i]->child)
                        hub_hc_release_resources(hub->ports[i]->child);

-       if (hcd->driver->reset_device)
-               hcd->driver->reset_device(hcd, udev);
+       if (hcd->driver->free_dev)
+                hcd->driver->free_dev(hcd, udev);
 }

It will free some resource and disable Slot from leaf to root. Later during resuming process
one by one Slot will enabled in xhci_discover_or_reset_device function.
This solution passed my test, but they are limited.

Pawel

>Are you able recreate  this issue with different xHC controllers or only with
>one specific xHCI?
>I try to recreate this issue but without result.
>
>Regards,
>Pawel
>
>>
>>
>>By the way, it's highly unclear if bug 220069 is caused by this spec
>>violation (I think it's not), but this is still very sloppy code.
>
>
>>
>>Regards,
>>Michal