linux-kernel - Re: [PATCH v4 2/2] USB: xhci: use urb hcpriv mempool for private data

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <2025052211-oxidizing-tannery-de3f@gregkh>
Date: Thu, 22 May 2025 10:32:40 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: David Wang <00107082@....com>
Cc: mathias.nyman@...el.com, oneukum@...e.com, stern@...land.harvard.edu,
	hminas@...opsys.com, rui.silva@...aro.org, jgross@...e.com,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/2] USB: xhci: use urb hcpriv mempool for private data

On Thu, May 22, 2025 at 03:10:10PM +0800, David Wang wrote:
> xhci keeps alloc/free private data for each enqueue/dequeue cycles,
> when using a USB webcam, allocation rate is ~250/s;
> when using a USB mic, allocation rate reaches ~1k/s;
> The more usb device in use, the higher allocation rate.
> 
> URB objects have longer lifespan than private data, hand over ownership
> of private data to urb can save lots of memory allocations over time.
> With this change, no extra memory allocation is needed during usages of
> USB webcam/mic.
> 
> Signed-off-by: David Wang <00107082@....com>
> ---
>  drivers/usb/host/xhci-mem.c  | 1 +
>  drivers/usb/host/xhci-ring.c | 3 +--
>  drivers/usb/host/xhci.c      | 8 +++-----
>  3 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
> index d698095fc88d..b19e41cf1c4c 100644
> --- a/drivers/usb/host/xhci-mem.c
> +++ b/drivers/usb/host/xhci-mem.c
> @@ -1745,6 +1745,7 @@ struct xhci_command *xhci_alloc_command_with_ctx(struct xhci_hcd *xhci,
>  
>  void xhci_urb_free_priv(struct urb_priv *urb_priv)
>  {
> +	WARN_ONCE(1, "xhci private data should be managed by urb");

You just crashed the kernel if this ever gets hit.  As you are saying
this should never be called, why is this function even present anymore?

This makes no sense :(

Again, NEVER add a WARN*() call to the kernel for something that it
should be handling properly on its own.  Otherwise you just lost all the
user's data when the box got rebooted (and if userspace can trigger
this, you just created a new CVE...)

thanks,

greg k-h