[v2] usb: hub: Set proper message when usb_hub_create_port_device() fails

[PATCH v2] usb: hub: Set proper message when usb_hub_create_port_device() fails

Posted by Chen-Yu Tsai 2 days, 7 hours ago

Right now when usb_hub_create_port_device() fails, it prints a separate
error message to say which port failed, but otherwise leaves 'message'
set to the default "out of memory", which is somewhat misleading.

Allocate some memory to put the custom formatted error message in and
use it as the error message. If the allocation fails, use a generic
version of the error message. The allocation is explicitly freed after
the error message is printed.

Variants of __free() were not used as it ends up mixing usage of __free()
and goto. Also, kfree_const() won't work as it uses is_kernel_rodata(),
which only returns true if the symbol or address is in the kernel image's
.rodata section; it doesn't work for module .rodata. And the USB
subsystem can be built as a module.

Assisted-by: LLM # local reviews
Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
Changes since v1:
- Explicitly track and free the allocated custom error message instead
  of using devm_kasprintf()
- Link to v1:
  https://lore.kernel.org/all/20260728100005.413868-1-wenst@chromium.org/

Sorry Andy, I ended up not using your __free(kfree_const) patch. My LLM
was telling me that it won't work correctly if CONFIG_USB=m.
---
 drivers/usb/core/hub.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 0e929a4c9fa1..398ae53cd8d7 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1480,6 +1480,8 @@ static int hub_configure(struct usb_hub *hub,
 	unsigned int pipe;
 	int maxp, ret, i;
 	char *message = "out of memory";
+	/* Used to hold and clean up kasprintf()-ed failure message */
+	char *msg_alloc = NULL;
 	unsigned unit_load;
 	unsigned full_load;
 	unsigned maxchild;
@@ -1756,8 +1758,11 @@ static int hub_configure(struct usb_hub *hub,
 	for (i = 0; i < maxchild; i++) {
 		ret = usb_hub_create_port_device(hub, i + 1);
 		if (ret < 0) {
-			dev_err(hub->intfdev,
-				"couldn't create port%d device.\n", i + 1);
+			msg_alloc = kasprintf(GFP_KERNEL, "couldn't create port%d device", i + 1);
+			if (msg_alloc)
+				message = msg_alloc;
+			else
+				message = "couldn't create port device";
 			break;
 		}
 	}
@@ -1792,6 +1797,7 @@ static int hub_configure(struct usb_hub *hub,
 fail:
 	dev_err(hub_dev, "config failed, %s (err %d)\n",
 			message, ret);
+	kfree(msg_alloc);
 	/* hub_disconnect() frees urb and descriptor */
 	return ret;
 }
-- 
2.55.0.1082.g2b9226bbc0-goog

Re: [PATCH v2] usb: hub: Set proper message when usb_hub_create_port_device() fails

Posted by Alan Stern 2 days ago

On Tue, Sep 22, 2026 at 04:05:41PM +0800, Chen-Yu Tsai wrote:
> Right now when usb_hub_create_port_device() fails, it prints a separate
> error message to say which port failed, but otherwise leaves 'message'
> set to the default "out of memory", which is somewhat misleading.
> 
> Allocate some memory to put the custom formatted error message in and
> use it as the error message. If the allocation fails, use a generic
> version of the error message. The allocation is explicitly freed after
> the error message is printed.
> 
> Variants of __free() were not used as it ends up mixing usage of __free()
> and goto. Also, kfree_const() won't work as it uses is_kernel_rodata(),
> which only returns true if the symbol or address is in the kernel image's
> .rodata section; it doesn't work for module .rodata. And the USB
> subsystem can be built as a module.
> 
> Assisted-by: LLM # local reviews
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
> Changes since v1:
> - Explicitly track and free the allocated custom error message instead
>   of using devm_kasprintf()
> - Link to v1:
>   https://lore.kernel.org/all/20260728100005.413868-1-wenst@chromium.org/
> 
> Sorry Andy, I ended up not using your __free(kfree_const) patch. My LLM
> was telling me that it won't work correctly if CONFIG_USB=m.
> ---
>  drivers/usb/core/hub.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
> index 0e929a4c9fa1..398ae53cd8d7 100644
> --- a/drivers/usb/core/hub.c
> +++ b/drivers/usb/core/hub.c
> @@ -1480,6 +1480,8 @@ static int hub_configure(struct usb_hub *hub,
>  	unsigned int pipe;
>  	int maxp, ret, i;
>  	char *message = "out of memory";
> +	/* Used to hold and clean up kasprintf()-ed failure message */

Comment is unnecessary.  People will see what the variable is used for 
by reading the code; it's not complicated.

> +	char *msg_alloc = NULL;
>  	unsigned unit_load;
>  	unsigned full_load;
>  	unsigned maxchild;
> @@ -1756,8 +1758,11 @@ static int hub_configure(struct usb_hub *hub,
>  	for (i = 0; i < maxchild; i++) {
>  		ret = usb_hub_create_port_device(hub, i + 1);
>  		if (ret < 0) {
> -			dev_err(hub->intfdev,
> -				"couldn't create port%d device.\n", i + 1);
> +			msg_alloc = kasprintf(GFP_KERNEL, "couldn't create port%d device", i + 1);

Why not use a fixed-size 64-byte buffer on the stack instead of 
allocating extra memory?

Alan Stern

> +			if (msg_alloc)
> +				message = msg_alloc;
> +			else
> +				message = "couldn't create port device";
>  			break;
>  		}
>  	}
> @@ -1792,6 +1797,7 @@ static int hub_configure(struct usb_hub *hub,
>  fail:
>  	dev_err(hub_dev, "config failed, %s (err %d)\n",
>  			message, ret);
> +	kfree(msg_alloc);
>  	/* hub_disconnect() frees urb and descriptor */
>  	return ret;
>  }
> -- 
> 2.55.0.1082.g2b9226bbc0-goog
> 
>