linux-kernel - Re: [PATCH v2 1/2] USB: core: add a memory pool to urb for 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: <484fe5f7.9d28.196cea2c6db.Coremail.00107082@163.com>
Date: Wed, 14 May 2025 19:51:36 +0800 (CST)
From: "David Wang" <00107082@....com>
To: "Oliver Neukum" <oneukum@...e.com>
Cc: mathias.nyman@...el.com, gregkh@...uxfoundation.org,
	stern@...land.harvard.edu, linux-usb@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] USB: core: add a memory pool to urb for
 host-controller private data


At 2025-05-14 19:23:02, "Oliver Neukum" <oneukum@...e.com> wrote:
>
>
>On 13.05.25 13:38, David Wang wrote:
>> ---
>
>Hi,
>
>still an issue after a second review.
>I should have noticed earlier.
>
>> --- a/drivers/usb/core/urb.c
>> +++ b/drivers/usb/core/urb.c
>> @@ -23,6 +23,7 @@ static void urb_destroy(struct kref *kref)
>>   
>>   	if (urb->transfer_flags & URB_FREE_BUFFER)
>>   		kfree(urb->transfer_buffer);
>> +	kfree(urb->hcpriv_mempool);
>
>What if somebody uses usb_init_urb()?

I am not quite sure about the concern here, do you mean somebody create a urb,
and then usb_init_urb() here, and never use urb_destroy to release it?

That would cause memory leak if urb_destroy is not called......But is this really possible?.

>  
>>   	kfree(urb);
>>   }
>> @@ -1037,3 +1038,25 @@ int usb_anchor_empty(struct usb_anchor *anchor)
>>   
>>   EXPORT_SYMBOL_GPL(usb_anchor_empty);
>>   
>> +/**
>> + * urb_hcpriv_mempool_zalloc - alloc memory from mempool for hcpriv
>> + * @urb: pointer to URB being used
>> + * @size: memory size requested by current host controller
>> + * @mem_flags: the type of memory to allocate
>> + *
>> + * Return: NULL if out of memory, otherwise memory are zeroed
>> + */
>> +void *urb_hcpriv_mempool_zalloc(struct urb *urb, size_t size, gfp_t mem_flags)
>> +{
>> +	if (urb->hcpriv_mempool_size < size) {
>> +		kfree(urb->hcpriv_mempool);
>> +		urb->hcpriv_mempool_size = size;
>> +		urb->hcpriv_mempool = kmalloc(size, mem_flags);
>
>That could use kzalloc().

The memory would be  set to 0 before returning to user, via memset,   no matter whether the memory is 
newly alloced or just reused.  I think using kmalloc is ok here.


Thanks
David

>
>	Regards
>		Oliver