[PATCH v1 2/3] usb: gadget: composite: fix WebUSB URL descriptor length handling

[Aristo Chen]

The bound passed to strnlen() subtracts the descriptor header twice:

	landing_page_length = strnlen(cdev->landing_page,
		sizeof(url_descriptor->URL)
		- WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH + landing_page_offset);

URL[] is already declared as U8_MAX - WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH
bytes, so it does not include the header, and subtracting the header
again leaves room for three bytes fewer than the descriptor can carry.

That is normally masked by the w_length handling below it, which folds
the host's requested length into the URL length:

	landing_page_length = w_length
		- WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH + landing_page_offset;

Doing so conflates three separate quantities, namely how much URL there
is, how large the descriptor is, and how many bytes the host asked for.
It gets all three wrong:

 - the emitted URL length becomes w_length - header, bounded by the
   request rather than by sizeof(url_descriptor->URL), so a host asking
   for w_length between 256 and 259 has up to 256 bytes copied into the
   252 byte URL[] and bLength, a u8, wraps to 0 or 3.  cdev->req->buf is
   USB_COMP_EP0_BUFSIZ bytes, so nothing outside the request buffer is
   touched, but the descriptor is malformed.

 - bLength ends up describing the transfer instead of the descriptor.
   WebUSB defines it as the size of the descriptor, so a full length
   landing page requested with w_length 4 must still report bLength 255
   while transferring four bytes; instead it reports 4.

 - for w_length below WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH the reply is
   the three byte header, which is more than the host's data stage.

Compute the URL length once, bounded only by sizeof(url_descriptor->URL),
build the descriptor from it, and shorten the reply alone with
min_t(u16, w_length, ...) the way the rest of composite_setup() already
does.  A 260 byte "https://"; landing page is now emitted as 252 URL
bytes with bLength 255, and short requests are answered with a correctly
sized descriptor truncated to what was asked for.

Fixes: 93c473948c58 ("usb: gadget: add WebUSB landing page support")
Signed-off-by: Aristo Chen <aristo.chen@xxxxxxxxxxxxx>
---
 drivers/usb/gadget/composite.c | 33 ++++++++++++++++++---------------
 1 file changed, 18 insertions(+), 15 deletions(-)

diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c
index df39e3487c1f..6c8e15faee6a 100644
--- a/drivers/usb/gadget/composite.c
+++ b/drivers/usb/gadget/composite.c
@@ -2151,7 +2151,7 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 		    w_index == WEBUSB_GET_URL &&
 		    w_value == WEBUSB_LANDING_PAGE_PRESENT &&
 		    ctrl->bRequest == cdev->b_webusb_vendor_code) {
-			unsigned int	landing_page_length;
+			unsigned int	url_length;
 			unsigned int	landing_page_offset;
 			struct webusb_url_descriptor *url_descriptor =
 					(struct webusb_url_descriptor *)cdev->req->buf;
@@ -2169,24 +2169,27 @@ composite_setup(struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
 				url_descriptor->bScheme = WEBUSB_URL_SCHEME_NONE;
 			}
 
-			landing_page_length = strnlen(cdev->landing_page,
-				sizeof(url_descriptor->URL)
-				- WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH + landing_page_offset);
-
-			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;
+			/*
+			 * The scheme prefix is encoded in bScheme and is not
+			 * emitted, so URL[] bounds what is left of the URL.
+			 */
+			url_length = strnlen(cdev->landing_page,
+					     sizeof(cdev->landing_page));
+			url_length -= landing_page_offset;
+			if (url_length > sizeof(url_descriptor->URL))
+				url_length = sizeof(url_descriptor->URL);
 
 			memcpy(url_descriptor->URL,
 				cdev->landing_page + landing_page_offset,
-				landing_page_length - landing_page_offset);
-			url_descriptor->bLength = landing_page_length
-				- landing_page_offset + WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH;
+				url_length);
+			url_descriptor->bLength = url_length
+				+ WEBUSB_URL_DESCRIPTOR_HEADER_LENGTH;
 
-			value = url_descriptor->bLength;
+			/*
+			 * bLength describes the descriptor, not the transfer,
+			 * so only the reply is shortened to what was asked for.
+			 */
+			value = min_t(u16, w_length, url_descriptor->bLength);
 
 			goto check_value;
 		}
-- 
2.53.0