Re: [PATCH v4 3/5] usb: xhci: sideband: allocate sideband ring segments from a dedicated pool
From: Wesley Cheng
Date: Tue Oct 13 2026 - 19:00:03 EST
On 10/13/2026 2:11 AM, Michal Pecio wrote:
On Mon, 12 Oct 2026 18:01:50 -0700, Wesley Cheng wrote:
On 10/5/2026 4:03 AM, Mathias Nyman wrote:
xhci is the one allocating the ring when adding an endpoint, and
telling xHC hardware to write to that ring. It seems more
approriate that xhci should be the one freeing the ring after it
told xHC hardware to drop the endpoint, and there no longer is a
risk on hardware accessing it.
Hi Mathias,
Sorry for the late response. Was out last week.
Specifically in the disconnect path (while audio offload is active),
when we utilize the usb_set_interface() path, it'll return early due
to the iface->unregistering check. This would lead to the
drop_endpoint not to be called, and rely on the xhci_free_dev() to
handle the transfer ring cleanup. This wouldn't work in the offload
situation, because qc_audio_offload is the only entity managing the
DMA pool, and that would be freed during the .disconnect callback,
which happens BEFORE xhci_free_dev().
Would you be more inclined to prefer a solution where we can defer
the DMA pool free'ing to the xHCI core during xhci_free_dev, or maybe
adding another sideband notifier (sb->notify_client) enum to tell the
client driver that it can release the pool?
I'm not Mathias but my opinion is the same as always: none of these
problems would exist if the sideband client allocated and freed all
transfer/event ring memory, and xhci-sideband only owned a mapping.
The mapping would be freed by xhci_sideband_unregister() or similar
calls, which should ensure that the xHC stops using the rings, and
memory would only be freed by the client driver after unregistering
the sideband and stopping the coprocessor. Nothing is ever UAF or
leaked, and ensuring this is *easy*. And if xhci-hcd randomly wants
to get rid of some endpoint, it simply stops using the mapping and
lets the sideband client clean it up two milliseconds later.
Hi Michal
Your feedback is always welcomed.
I guess my intention was to see if we could leverage the existing memory operations within xHCI as much as we could. The only concern I would have with having the client own all the memory operations/allocations is that we would need to carefully maintain the transitional phases between offloaded and non-offloaded scenarios to be handled properly. At least in the audio offload case, we're only offloading data to the DSP when audio is active, which can be for a short duration, and we have proper checks and locking already within the xHCI mem/core layers.
One thing I'm not entirely sure about is how we could create this
mapping. Using bare iommu_map() creates a hard dependency on IOMMU.
Documentation/core-api/dma-attributes.rst mentions possibility of
using generic dma_map_() helpers on multiple devices, so maybe this
can work, but I haven't seen existing examples of such use.
And there seems to be one more potential problem case, which hasn't
been mentioned in this thread AFAIK: if xHC resume failes, xhci-hcd
frees its private data and tell USB core to reset-resume devices.
Looks like another opportunity for UAF.
Since I did take a look at the other issue that probably reminded you of this, without getting on to a tangent subject, it really broke down to two scenarios:
1. reset_resume during bus resume
2. usb_device_reset for recovery
In both situations there are requirements that when those happen/trigger, technically, the bus should be completely idle. For #1, since we're coming from a bus suspend transition, there should be no pending URBs in flight, so if reset_resume happened, xfer rings aren't being utilized. For #2, there were some changes that weren't properly handling the pre and post reset callbacks, leading to problems in that situation. I think that series showed us that there are corner cases where referencing vdev could potentially be unreliable. I'm going to assume this has no relationship to the client driver owning memory, please correct me if I'm wrong. :)
Thanks
Wesley Cheng