Re: [PATCH v1 1/3] usb: gadget: configfs: fix WebUSB landing page missing NUL terminator

[Alan Stern]

On Mon, Aug 31, 2026 at 03:39:37PM +0000, Aristo Chen wrote:
> landing_page is sized WEBUSB_URL_RAW_MAX_LENGTH, which is exactly the
> length of the longest URL that can be represented in a WebUSB URL
> descriptor (U8_MAX - WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH + 8 == 260),
> leaving no room for a NUL terminator.
> 
> webusb_landingPage_store() bounds the URL with
> 
> 	if (l > sizeof(gi->landing_page))
> 
> so l == 260 is accepted, and for a "https://"; URL the second bound
> allows 260 as well.  memcpy_and_pad() degenerates to a plain memcpy()
> when dest_len == count, so nothing terminates the string:
> 
> 	printf '%s' "https://$(printf 'A%.0s' $(seq 252))" > webusb/landingPage
> 
> configfs runs store() once per write(), so the 260 bytes have to reach
> it in a single write to hit the case.
> 
> webusb_landingPage_show() then does sysfs_emit(page, "%s\n", ...), which
> reads past the end of the array.  What follows landing_page is the
> padding in front of the spinlock member, three bytes of it in the
> layout here, and kzalloc() left that padding zero, so the read stops
> there and the attribute happens to return exactly the bytes that were
> written.  The over-read is harmless today only by accident of the
> layout: the terminator is never written, and nothing keeps a new member
> or a different configuration from putting live data where the zeroed
> padding currently sits.
> 
> A 260 byte URL is legitimate: after stripping "https://"; it yields a
> 252 byte URL descriptor payload, or bLength == U8_MAX exactly.  So
> rather than rejecting it, give the buffers room for the terminator and
> keep WEBUSB_URL_RAW_MAX_LENGTH as what its name says, a maximum URL
> length.  The explicit bound in webusb_landingPage_store() now uses that
> macro instead of sizeof(), since the buffer is deliberately one byte
> larger than the longest URL it may hold.
> 
> Fixes: 93c473948c58 ("usb: gadget: add WebUSB landing page support")
> Signed-off-by: Aristo Chen <aristo.chen@xxxxxxxxxxxxx>
> ---

A somewhat verbose description, but okay.

Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>

>  drivers/usb/gadget/configfs.c | 6 +++---
>  include/linux/usb/composite.h | 2 +-
>  include/linux/usb/webusb.h    | 5 ++++-
>  3 files changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/usb/gadget/configfs.c b/drivers/usb/gadget/configfs.c
> index 51df6d1d1487..4bc95f4b6670 100644
> --- a/drivers/usb/gadget/configfs.c
> +++ b/drivers/usb/gadget/configfs.c
> @@ -55,7 +55,7 @@ struct gadget_info {
>  	bool use_webusb;
>  	u16 bcd_webusb_version;
>  	u8 b_webusb_vendor_code;
> -	char landing_page[WEBUSB_URL_RAW_MAX_LENGTH];
> +	char landing_page[WEBUSB_URL_RAW_MAX_LENGTH + 1];
>  
>  	spinlock_t spinlock;
>  	bool unbind;
> @@ -1072,7 +1072,7 @@ static ssize_t webusb_landingPage_store(struct config_item *item, const char *pa
>  		++bytes_to_strip;
>  	}
>  
> -	if (l > sizeof(gi->landing_page)) {
> +	if (l > WEBUSB_URL_RAW_MAX_LENGTH) {
>  		pr_err("webusb: landingPage URL too long\n");
>  		return -EINVAL;
>  	}
> @@ -1742,7 +1742,7 @@ static int configfs_composite_bind(struct usb_gadget *gadget,
>  		cdev->use_webusb = true;
>  		cdev->bcd_webusb_version = gi->bcd_webusb_version;
>  		cdev->b_webusb_vendor_code = gi->b_webusb_vendor_code;
> -		memcpy(cdev->landing_page, gi->landing_page, WEBUSB_URL_RAW_MAX_LENGTH);
> +		memcpy(cdev->landing_page, gi->landing_page, sizeof(cdev->landing_page));
>  	}
>  
>  	if (gi->use_os_desc) {
> diff --git a/include/linux/usb/composite.h b/include/linux/usb/composite.h
> index c18041fafa52..0621a6a5cc57 100644
> --- a/include/linux/usb/composite.h
> +++ b/include/linux/usb/composite.h
> @@ -472,7 +472,7 @@ struct usb_composite_dev {
>  	/* WebUSB */
>  	u16				bcd_webusb_version;
>  	u8				b_webusb_vendor_code;
> -	char				landing_page[WEBUSB_URL_RAW_MAX_LENGTH];
> +	char				landing_page[WEBUSB_URL_RAW_MAX_LENGTH + 1];
>  	unsigned int			use_webusb:1;
>  
>  	/* private: */
> diff --git a/include/linux/usb/webusb.h b/include/linux/usb/webusb.h
> index fe43020b4a48..a3febe726911 100644
> --- a/include/linux/usb/webusb.h
> +++ b/include/linux/usb/webusb.h
> @@ -68,12 +68,15 @@ struct webusb_url_descriptor {
>  } __packed;
>  
>  /*
> - * Buffer size to hold the longest URL that can be in an URL descriptor
> + * Length of the longest URL that can be in an URL descriptor
>   *
>   * The descriptor can be U8_MAX  bytes long.
>   * WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH bytes are used for a header.
>   * Since the longest prefix that might be stripped is "https://";, we may accommodate an additional
>   * 8 bytes.
> + *
> + * Note that this is a string length and not a buffer size: a buffer holding such
> + * a URL needs one more byte for the NUL terminator.
>   */
>  #define WEBUSB_URL_RAW_MAX_LENGTH (U8_MAX - WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH + 8)
>  
> -- 
> 2.53.0
> 
>