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

[Alan Stern]

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@xxxxxxxxxxxx>
> ---
> 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@xxxxxxxxxxxx/
> 
> 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
> 
>