linux-kernel - Re: [PATCH v3 1/2] USB: core: add a memory pool to urb caching host-controller 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: <49228bf7.a1d1.196f303aa15.Coremail.00107082@163.com>
Date: Wed, 21 May 2025 21:23:45 +0800 (CST)
From: "David Wang" <00107082@....com>
To: "Mathias Nyman" <mathias.nyman@...el.com>
Cc: gregkh@...uxfoundation.org, oneukum@...e.com, stern@...land.harvard.edu,
	linux-usb@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/2] USB: core: add a memory pool to urb caching
 host-controller private data


At 2025-05-21 20:28:17, "Mathias Nyman" <mathias.nyman@...el.com> wrote:
>On 17.5.2025 11.38, David Wang wrote:
>> ---
>> Changes since v2:
>> 1. activat the pool only when the urb object is created via
>> usb_alloc_urb()
>> Thanks to Oliver Neukum <oneukum@...e.com>'s review.
>> ---
>> URB objects have long lifecycle, an urb can be reused between
>> submit loops; The private data needed by some host controller
>> has very short lifecycle, the memory is alloced when enqueue, and
>> released when dequeue. For example, on a system with xhci, in
>> xhci_urb_enqueue:
>> Using a USB webcam would have ~250/s memory allocation;
>> Using a USB mic would have ~1K/s memory allocation;
>> 
>> High frequent allocations for host-controller private data can be
>> avoided if urb take over the ownership of memory, the memory then shares
>> the longer lifecycle with urb objects.
>> 
>> Add a mempool to urb for hcpriv usage, the mempool only holds one block
>> of memory and grows when larger size is requested.
>> 
>> The mempool is activated only when the URB object is created via
>> usb_alloc_urb() in case some drivers create a URB object by other
>> means and manage it lifecycle without corresponding usb_free_urb.
>
>Won't this still allocate a lot of unnecessary memory for the roothub urbs?
>i.e. the ones queued with rh_urb_enqueue(hcd, urb).
>The host drivers don't use the urb->hcpriv of those URBs.
>
The mempool slot is alloced on demand when hcd request private data with its urb.
If a urb is  ever used by hcd and the  hcd requests private data with it, a  memory would be alloced
and this memory will not be released until the urb is destroyed.

If those URB keeps being reused, than the mempool slot within it can be reused by hcds which get hold on it.

And yes, it a URB is used only once or used very unfrequently, and hcd needs private data with it, the mempool
slot would be alloced with it and would be a very useless mempool slot, wasting memory.

>This would be the roothub status URB, and every hub request sent
>during device enumeration for devices connected to the roothub.

>
>Is this whole URB hcpriv mempool addition an actual issue that needs fixing?

NO, not an issue, just meant to avoid memory allocations, lots of allocation, to theoretically  improve things..

>
>If yes then I still think it's better to do it in the host driver like I
>described earlier. I don't think it will be too complex
>
>Thanks
>-Mathias
>
>