Re: [PATCH] xhci: sideband: fix ring sg table pages leak

[Greg KH]

On Tue, Jun 02, 2026 at 11:43:27AM +0800, Xu Rao wrote:
> xhci_ring_to_sgtable() allocates a temporary pages array and
> uses it to build the returned sg_table with
> sg_alloc_table_from_pages().
> 
> The error paths free the pages array, but the success path
> returns the sg_table without freeing it. This leaks the temporary
> array every time a sideband client gets an endpoint or event ring
> buffer.
> 
> Free the pages array after sg_alloc_table_from_pages() succeeds.
> The returned sg_table has its own scatterlist entries and does not
> depend on the temporary array after construction.
> 
> Fixes: de66754e9f80 ("xhci: sideband: add initial api to register a secondary interrupter entity")
> 
> Signed-off-by: Xu Rao <raoxu@xxxxxxxxxxxxx>
> ---
>  drivers/usb/host/xhci-sideband.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/host/xhci-sideband.c b/drivers/usb/host/xhci-sideband.c
> index 23153e136d4b..a5deeee4d5dc 100644
> --- a/drivers/usb/host/xhci-sideband.c
> +++ b/drivers/usb/host/xhci-sideband.c
> @@ -58,6 +58,8 @@ xhci_ring_to_sgtable(struct xhci_sideband *sb, struct xhci_ring *ring)
>  	if (sg_alloc_table_from_pages(sgt, pages, n_pages, 0, sz, GFP_KERNEL))
>  		goto err;
> 
> +	kvfree(pages);

This looks very odd, how was this tested?  If
sg_alloc_table_from_pages() succeeds, then the table has the pages in
it, you don't want to free them again, right?

And if this is such a major bug, why hasn't anyone noticed the leak yet?

thanks,

greg k-h