[PATCH] usb: gadget: composite: fix integer underflow in WebUSB GET_URL handling

Jeremy Erazo posted 1 patch 1 month ago
There is a newer version of this series
drivers/usb/gadget/composite.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
[PATCH] usb: gadget: composite: fix integer underflow in WebUSB GET_URL handling
Posted by Jeremy Erazo 1 month ago
The WebUSB GET_URL handler in composite_setup() narrows
landing_page_length to fit the host-supplied wLength using

	landing_page_length = w_length
		- WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH + landing_page_offset;

If wLength is smaller than WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH the
unsigned subtraction wraps, and the subsequent

	memcpy(url_descriptor->URL,
	       cdev->landing_page + landing_page_offset,
	       landing_page_length - landing_page_offset);

ends up copying close to UINT_MAX bytes from cdev->landing_page into
cdev->req->buf.  KASAN reports a slab-out-of-bounds in composite_setup
on the kmalloc-2k gadget_info allocation, and FORTIFY_SOURCE traps the
memcpy as a 4294967293-byte field-spanning write into
url_descriptor->URL (size 252).

A USB host can reach this from a single SETUP packet against any
gadget that has webusb/use=1 and a landingPage configured.

Handle the small-wLength case before the math: when the host requested
fewer bytes than the URL descriptor header, only the header is
meaningful and no URL bytes need to be copied.  Setting
landing_page_length to landing_page_offset makes the existing memcpy a
no-op and leaves the descriptor returned to the host unchanged for all
larger wLength values.

Reported-by: Jeremy Erazo <mendozayt13@gmail.com>
Signed-off-by: Jeremy Erazo <mendozayt13@gmail.com>
---
 drivers/usb/gadget/composite.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index a902184bd..dc3664374 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -2172,7 +2172,10 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 				sizeof(url_descriptor->URL)
 				- WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH + landing_page_offset);
 
-			if (w_length < WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH + landing_page_length)
+			if (w_length < WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH)
+				landing_page_length = landing_page_offset;
+			else if (w_length <
+				 WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH + landing_page_length)
 				landing_page_length = w_length
 				- WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH + landing_page_offset;
 
-- 
2.53.0
Re: [PATCH] usb: gadget: composite: fix integer underflow in WebUSB GET_URL handling
Posted by Greg Kroah-Hartman 1 month ago
On Tue, May 12, 2026 at 01:43:43AM +0000, Jeremy Erazo wrote:
> The WebUSB GET_URL handler in composite_setup() narrows
> landing_page_length to fit the host-supplied wLength using
> 
> 	landing_page_length = w_length
> 		- WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH + landing_page_offset;
> 
> If wLength is smaller than WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH the
> unsigned subtraction wraps, and the subsequent
> 
> 	memcpy(url_descriptor->URL,
> 	       cdev->landing_page + landing_page_offset,
> 	       landing_page_length - landing_page_offset);
> 
> ends up copying close to UINT_MAX bytes from cdev->landing_page into
> cdev->req->buf.  KASAN reports a slab-out-of-bounds in composite_setup
> on the kmalloc-2k gadget_info allocation, and FORTIFY_SOURCE traps the
> memcpy as a 4294967293-byte field-spanning write into
> url_descriptor->URL (size 252).
> 
> A USB host can reach this from a single SETUP packet against any
> gadget that has webusb/use=1 and a landingPage configured.
> 
> Handle the small-wLength case before the math: when the host requested
> fewer bytes than the URL descriptor header, only the header is
> meaningful and no URL bytes need to be copied.  Setting
> landing_page_length to landing_page_offset makes the existing memcpy a
> no-op and leaves the descriptor returned to the host unchanged for all
> larger wLength values.
> 
> Reported-by: Jeremy Erazo <mendozayt13@gmail.com>
> Signed-off-by: Jeremy Erazo <mendozayt13@gmail.com>

You don't need a reported-by when you are the author and sign off on
something.

What commit id does this fix?  Why not backport it to stable kernels?

thanks,

greg k-h
Re: [PATCH] usb: gadget: composite: fix integer underflow in WebUSB GET_URL handling
Posted by Sebastian EM 1 month ago
Hi Greg,

Thanks for the review.

You are right; the self Reported-by tag does not belong there, so I dropped
it in v2.

The introducing commit is:

93c473948c58 ("usb: gadget: add WebUSB landing page support")

I also added:

Cc: stable@vger.kernel.org

since the issue was introduced with the WebUSB GET_URL handling path and
the fix is a small bounds/underflow fix suitable for stable kernels.

v2 is attached as a plain patch:

0001-v2-usb-gadget-composite-fix-integer-underflow-in-WebUSB-GET_URL-handling.patch

Thanks,
Jeremy

El mar, 12 may 2026 a las 0:40, Greg Kroah-Hartman (<
gregkh@linuxfoundation.org>) escribió:

> On Tue, May 12, 2026 at 01:43:43AM +0000, Jeremy Erazo wrote:
> > The WebUSB GET_URL handler in composite_setup() narrows
> > landing_page_length to fit the host-supplied wLength using
> >
> >       landing_page_length = w_length
> >               - WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH +
> landing_page_offset;
> >
> > If wLength is smaller than WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH the
> > unsigned subtraction wraps, and the subsequent
> >
> >       memcpy(url_descriptor->URL,
> >              cdev->landing_page + landing_page_offset,
> >              landing_page_length - landing_page_offset);
> >
> > ends up copying close to UINT_MAX bytes from cdev->landing_page into
> > cdev->req->buf.  KASAN reports a slab-out-of-bounds in composite_setup
> > on the kmalloc-2k gadget_info allocation, and FORTIFY_SOURCE traps the
> > memcpy as a 4294967293-byte field-spanning write into
> > url_descriptor->URL (size 252).
> >
> > A USB host can reach this from a single SETUP packet against any
> > gadget that has webusb/use=1 and a landingPage configured.
> >
> > Handle the small-wLength case before the math: when the host requested
> > fewer bytes than the URL descriptor header, only the header is
> > meaningful and no URL bytes need to be copied.  Setting
> > landing_page_length to landing_page_offset makes the existing memcpy a
> > no-op and leaves the descriptor returned to the host unchanged for all
> > larger wLength values.
> >
> > Reported-by: Jeremy Erazo <mendozayt13@gmail.com>
> > Signed-off-by: Jeremy Erazo <mendozayt13@gmail.com>
>
> You don't need a reported-by when you are the author and sign off on
> something.
>
> What commit id does this fix?  Why not backport it to stable kernels?
>
> thanks,
>
> greg k-h
>
From 4d65e386a32c27c3ee46a9bf3f3807f89805b5b0 Mon Sep 17 00:00:00 2001
From: Jeremy Erazo <mendozayt13@gmail.com>
Date: Tue, 12 May 2026 01:31:16 +0000
Subject: [PATCH v2] usb: gadget: composite: fix integer underflow in WebUSB
 GET_URL handling

The WebUSB GET_URL handler in composite_setup() narrows
landing_page_length to fit the host-supplied wLength using

	landing_page_length = w_length
		- WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH + landing_page_offset;

If wLength is smaller than WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH the
unsigned subtraction wraps, and the subsequent

	memcpy(url_descriptor->URL,
	       cdev->landing_page + landing_page_offset,
	       landing_page_length - landing_page_offset);

ends up copying close to UINT_MAX bytes from cdev->landing_page into
cdev->req->buf.  KASAN reports a slab-out-of-bounds in composite_setup
on the kmalloc-2k gadget_info allocation, and FORTIFY_SOURCE traps the
memcpy as a 4294967293-byte field-spanning write into
url_descriptor->URL (size 252).

A USB host can reach this from a single SETUP packet against any
gadget that has webusb/use=1 and a landingPage configured.

Handle the small-wLength case before the math: when the host requested
fewer bytes than the URL descriptor header, only the header is
meaningful and no URL bytes need to be copied.  Setting
landing_page_length to landing_page_offset makes the existing memcpy a
no-op and leaves the descriptor returned to the host unchanged for all
larger wLength values.

Fixes: 93c473948c58 ("usb: gadget: add WebUSB landing page support")
Cc: stable@vger.kernel.org
Signed-off-by: Jeremy Erazo <mendozayt13@gmail.com>
---
 drivers/usb/gadget/composite.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index a902184bd..dc3664374 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -2172,7 +2172,10 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 				sizeof(url_descriptor->URL)
 				- WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH + landing_page_offset);
 
-			if (w_length < WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH + landing_page_length)
+			if (w_length < WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH)
+				landing_page_length = landing_page_offset;
+			else if (w_length <
+				 WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH + landing_page_length)
 				landing_page_length = w_length
 				- WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH + landing_page_offset;
 
-- 
2.53.0

Re: [PATCH] usb: gadget: composite: fix integer underflow in WebUSB GET_URL handling
Posted by Greg Kroah-Hartman 1 month ago
On Tue, May 12, 2026 at 06:18:54AM -0500, Sebastian EM wrote:
> Hi Greg,
> 
> Thanks for the review.
> 
> You are right; the self Reported-by tag does not belong there, so I dropped
> it in v2.
> 
> The introducing commit is:
> 
> 93c473948c58 ("usb: gadget: add WebUSB landing page support")
> 
> I also added:
> 
> Cc: stable@vger.kernel.org
> 
> since the issue was introduced with the WebUSB GET_URL handling path and
> the fix is a small bounds/underflow fix suitable for stable kernels.
> 
> v2 is attached as a plain patch:
> 
> 0001-v2-usb-gadget-composite-fix-integer-underflow-in-WebUSB-GET_URL-handling.patch

We can't take patches as attachments, just send this as a normal v2
patch.

thanks,

greg k-h