Re: [PATCH 3/9] usb: xhci: factor out roothub bandwidth cleanup

[Michal Pecio <michal.pecio@gmail.com>]

On Tue, 31 Mar 2026 12:34:36 +0300, Neronin, Niklas wrote:
> > This loop used to run before xhci_free_virt_devices_depth_first(),
> > but now it will run after. It seems that the endpoints here are
> > virt_ep which also should be gone already, so this loop likely does
> > nothing (empty list) or writes to virtual devices after free
> > (somebody forgot to unlink some endpoints from the list).  
> 
> In my testing, when xhci_rh_bw_cleanup() is called after
> xhci_free_virt_devices_depth_first() in xhci_resume(), all related
> resources have already been freed.
> That said, I have chosen to keep the existing freeing in this patch
> set. Removing it would introduce an additional behavioral change and a
> potential regression point, which I prefer to avoid at this stage.

Well, reordering these loops is also a potential behavior change.

As vdevs and tt_infos are closely tied together, I think it would make
sense for one function to free all of that stuff.

A non-behavior-changing way of doing it would be to extract the three
existing loops to such a function, in the exact order they run today.

> Do we trust xhci_free_virt_devices_depth_first() to work correctly?
> If yes then it seems this whole function is unnecessary.  

I think it's correct now, in the sense that all vdevs are removed and
there is no UAF along the way. Although a few months ago an unrelated
patch did break it unexpectedly, including UAF in some edge cases.

In principle it should be possible to drop separate tt_info cleanup,
because removing vdevs achieves the same. And if it doesn't then things
would also be broken under normal operation, not only suspend, as the
same xhci_free_virt_device() is used in both situations.

Regards,
Michal