Re: [PATCH v4 3/5] usb: xhci: sideband: allocate sideband ring segments from a dedicated pool

[Michal Pecio]

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.

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.

Regards,
Michal