usb: usbip: add NULL check after calloc()

[PATCH] usb: usbip: add NULL check after calloc()

Posted by longlong yan 3 weeks, 3 days ago

Two calloc() calls in the usbip driver lack NULL return checks, leading
to potential NULL pointer dereferences on allocation failure:

1. usbipd.c do_standalone_mode(): the allocated `fds` array is
   immediately dereferenced in the following for-loop via fds[i].fd
   without checking for NULL.

2. usbip_host_common.c usbip_exported_device_new(): the allocated
   `edev` is immediately dereferenced via edev->sudev without checking
   for NULL.

Add NULL checks after each calloc(), returning -1 in do_standalone_mode()
and using the existing goto err path in usbip_exported_device_new(),
consistent with the error handling already present in both functions.

Signed-off-by: longlong yan <yanlonglong@kylinos.cn>
---
 tools/usb/usbip/libsrc/usbip_host_common.c | 2 ++
 tools/usb/usbip/src/usbipd.c               | 4 ++++
 2 files changed, 6 insertions(+)

diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c b/tools/usb/usbip/libsrc/usbip_host_common.c
index 01599cb2fa7b..8ad367e09e78 100644
--- a/tools/usb/usbip/libsrc/usbip_host_common.c
+++ b/tools/usb/usbip/libsrc/usbip_host_common.c
@@ -71,6 +71,8 @@ struct usbip_exported_device *usbip_exported_device_new(
 	int i;
 
 	edev = calloc(1, sizeof(struct usbip_exported_device));
+	if (!edev)
+		goto err;
 
 	edev->sudev =
 		udev_device_new_from_syspath(udev_context, sdevpath);
diff --git a/tools/usb/usbip/src/usbipd.c b/tools/usb/usbip/src/usbipd.c
index 3e22b651c754..cc707dea2882 100644
--- a/tools/usb/usbip/src/usbipd.c
+++ b/tools/usb/usbip/src/usbipd.c
@@ -544,6 +544,10 @@ static int do_standalone_mode(int daemonize, int ipv4, int ipv6)
 	dbg("listening on %d address%s", nsockfd, (nsockfd == 1) ? "" : "es");
 
 	fds = calloc(nsockfd, sizeof(struct pollfd));
+	if (!fds) {
+		err("calloc for fds");
+		return -1;
+	}
 	for (i = 0; i < nsockfd; i++) {
 		fds[i].fd = sockfdlist[i];
 		fds[i].events = POLLIN;
-- 
2.43.0

Re: [PATCH] usb: usbip: add NULL check after calloc()

Posted by Greg KH 3 weeks, 3 days ago

On Wed, Sep 02, 2026 at 03:09:31PM +0800, longlong yan wrote:
> Two calloc() calls in the usbip driver lack NULL return checks, leading
> to potential NULL pointer dereferences on allocation failure:

This is in userspace, not in the "driver".

And how do you get a failure for calloc() in userspace?

> 1. usbipd.c do_standalone_mode(): the allocated `fds` array is
>    immediately dereferenced in the following for-loop via fds[i].fd
>    without checking for NULL.
> 
> 2. usbip_host_common.c usbip_exported_device_new(): the allocated
>    `edev` is immediately dereferenced via edev->sudev without checking
>    for NULL.
> 
> Add NULL checks after each calloc(), returning -1 in do_standalone_mode()
> and using the existing goto err path in usbip_exported_device_new(),
> consistent with the error handling already present in both functions.
> 
> Signed-off-by: longlong yan <yanlonglong@kylinos.cn>
> ---
>  tools/usb/usbip/libsrc/usbip_host_common.c | 2 ++
>  tools/usb/usbip/src/usbipd.c               | 4 ++++
>  2 files changed, 6 insertions(+)
> 
> diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c b/tools/usb/usbip/libsrc/usbip_host_common.c
> index 01599cb2fa7b..8ad367e09e78 100644
> --- a/tools/usb/usbip/libsrc/usbip_host_common.c
> +++ b/tools/usb/usbip/libsrc/usbip_host_common.c
> @@ -71,6 +71,8 @@ struct usbip_exported_device *usbip_exported_device_new(
>  	int i;
>  
>  	edev = calloc(1, sizeof(struct usbip_exported_device));
> +	if (!edev)
> +		goto err;
>  
>  	edev->sudev =
>  		udev_device_new_from_syspath(udev_context, sdevpath);
> diff --git a/tools/usb/usbip/src/usbipd.c b/tools/usb/usbip/src/usbipd.c
> index 3e22b651c754..cc707dea2882 100644
> --- a/tools/usb/usbip/src/usbipd.c
> +++ b/tools/usb/usbip/src/usbipd.c
> @@ -544,6 +544,10 @@ static int do_standalone_mode(int daemonize, int ipv4, int ipv6)
>  	dbg("listening on %d address%s", nsockfd, (nsockfd == 1) ? "" : "es");
>  
>  	fds = calloc(nsockfd, sizeof(struct pollfd));
> +	if (!fds) {
> +		err("calloc for fds");
> +		return -1;

I don't think you tested this :(

You leak stuff here, right?

thanks,

greg k-h

Re: [PATCH] usb: usbip: add NULL check after calloc()

Posted by Shuah Khan 3 weeks ago

On 9/2/26 02:42, Greg KH wrote:
> On Wed, Sep 02, 2026 at 03:09:31PM +0800, longlong yan wrote:
>> Two calloc() calls in the usbip driver lack NULL return checks, leading
>> to potential NULL pointer dereferences on allocation failure:
> 
> This is in userspace, not in the "driver".
> 
> And how do you get a failure for calloc() in userspace?
> 
>> 1. usbipd.c do_standalone_mode(): the allocated `fds` array is
>>     immediately dereferenced in the following for-loop via fds[i].fd
>>     without checking for NULL.
>>
>> 2. usbip_host_common.c usbip_exported_device_new(): the allocated
>>     `edev` is immediately dereferenced via edev->sudev without checking
>>     for NULL.
>>
>> Add NULL checks after each calloc(), returning -1 in do_standalone_mode()
>> and using the existing goto err path in usbip_exported_device_new(),
>> consistent with the error handling already present in both functions.
>>
>> Signed-off-by: longlong yan <yanlonglong@kylinos.cn>
>> ---
>>   tools/usb/usbip/libsrc/usbip_host_common.c | 2 ++
>>   tools/usb/usbip/src/usbipd.c               | 4 ++++
>>   2 files changed, 6 insertions(+)
>>
>> diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c b/tools/usb/usbip/libsrc/usbip_host_common.c
>> index 01599cb2fa7b..8ad367e09e78 100644
>> --- a/tools/usb/usbip/libsrc/usbip_host_common.c
>> +++ b/tools/usb/usbip/libsrc/usbip_host_common.c
>> @@ -71,6 +71,8 @@ struct usbip_exported_device *usbip_exported_device_new(
>>   	int i;
>>   
>>   	edev = calloc(1, sizeof(struct usbip_exported_device));
>> +	if (!edev)
>> +		goto err;
>>   
>>   	edev->sudev =
>>   		udev_device_new_from_syspath(udev_context, sdevpath);
>> diff --git a/tools/usb/usbip/src/usbipd.c b/tools/usb/usbip/src/usbipd.c
>> index 3e22b651c754..cc707dea2882 100644
>> --- a/tools/usb/usbip/src/usbipd.c
>> +++ b/tools/usb/usbip/src/usbipd.c
>> @@ -544,6 +544,10 @@ static int do_standalone_mode(int daemonize, int ipv4, int ipv6)
>>   	dbg("listening on %d address%s", nsockfd, (nsockfd == 1) ? "" : "es");
>>   
>>   	fds = calloc(nsockfd, sizeof(struct pollfd));
>> +	if (!fds) {
>> +		err("calloc for fds");
>> +		return -1;
> 
> I don't think you tested this :(
> 
> You leak stuff here, right?
> 

Thank you Greg for the review.


longlong yan, I am very reluctant to take usbip patches without
solid evidence of a real bug that can be reproduced and the patch
fixes it.

I am seeing a few too many usbip patches these days that don't fix
any bugs.

thanks,
-- Shuah