PATCH 1/1: greybus/usb: handle unspecified lengths in hub_control

PATCH 1/1: greybus/usb: handle unspecified lengths in hub_control

[Jose A. Perez de Azpillaga]

From 1e099b581fe475905509b9d600015ea2500b8cf8 Mon Sep 17 00:00:00 2001
From: "Jose A. Perez de Azpillaga" <azpijr@gmail.com>
Date: Wed, 11 Feb 2026 22:54:40 +0100
Subject: [PATCH] greybus/usb: handle unspecified lengths in hub_control

Fixes the FIXME in hub_control where response length was not handled correctly.

Signed-off-by: Jose A. Perez de Azpillaga <azpijr@gmail.com>
---
 drivers/staging/greybus/usb.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/greybus/usb.c b/drivers/staging/greybus/usb.c
index 475f24f20cd4..f5f5a4863ddc 100644
--- a/drivers/staging/greybus/usb.c
+++ b/drivers/staging/greybus/usb.c
@@ -105,8 +105,10 @@ static int hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, u16 wIndex,
 	size_t response_size;
 	int ret;
 
-	/* FIXME: handle unspecified lengths */
-	response_size = sizeof(*response) + wLength;
+	/* Calculate expected response size */
+	response_size = sizeof(*response);
+	if (wLength)
+		response_size += wLength;
 
 	operation = gb_operation_create(dev->connection,
 					GB_USB_TYPE_HUB_CONTROL,
@@ -127,9 +129,13 @@ static int hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, u16 wIndex,
 		goto out;
 
 	if (wLength) {
-		/* Greybus core has verified response size */
-		response = operation->response->payload;
-		memcpy(buf, response->buf, wLength);
+		size_t actual_size = operation->response->payload_size - sizeof(*response);
+		size_t copy_size = min(wLength, actual_size);
+
+		if (copy_size) {
+			response = operation->response->payload;
+			memcpy(buf, response->buf, copy_size);
+		}
 	}
 out:
 	gb_operation_put(operation);
-- 
2.51.0

Re: [greybus-dev] PATCH 1/1: greybus/usb: handle unspecified lengths in hub_control

[Greg KH]

On Wed, Feb 11, 2026 at 11:02:17PM +0100, Jose A. Perez de Azpillaga wrote:
> >From 1e099b581fe475905509b9d600015ea2500b8cf8 Mon Sep 17 00:00:00 2001
> From: "Jose A. Perez de Azpillaga" <azpijr@gmail.com>
> Date: Wed, 11 Feb 2026 22:54:40 +0100
> Subject: [PATCH] greybus/usb: handle unspecified lengths in hub_control

Something went wrong with your email client to include this in the
changelog area.  Perhaps use git send-email instead?

> 
> Fixes the FIXME in hub_control where response length was not handled correctly.

Can you wrap these lines at 72 columns like the editor asks?

> 
> Signed-off-by: Jose A. Perez de Azpillaga <azpijr@gmail.com>
> ---
>  drivers/staging/greybus/usb.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/staging/greybus/usb.c b/drivers/staging/greybus/usb.c
> index 475f24f20cd4..f5f5a4863ddc 100644
> --- a/drivers/staging/greybus/usb.c
> +++ b/drivers/staging/greybus/usb.c
> @@ -105,8 +105,10 @@ static int hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, u16 wIndex,
>  	size_t response_size;
>  	int ret;
>  
> -	/* FIXME: handle unspecified lengths */
> -	response_size = sizeof(*response) + wLength;
> +	/* Calculate expected response size */
> +	response_size = sizeof(*response);
> +	if (wLength)
> +		response_size += wLength;

How is this handling an unspecified length?


>  
>  	operation = gb_operation_create(dev->connection,
>  					GB_USB_TYPE_HUB_CONTROL,
> @@ -127,9 +129,13 @@ static int hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, u16 wIndex,
>  		goto out;
>  
>  	if (wLength) {
> -		/* Greybus core has verified response size */
> -		response = operation->response->payload;
> -		memcpy(buf, response->buf, wLength);
> +		size_t actual_size = operation->response->payload_size - sizeof(*response);
> +		size_t copy_size = min(wLength, actual_size);
> +
> +		if (copy_size) {
> +			response = operation->response->payload;
> +			memcpy(buf, response->buf, copy_size);
> +		}

Sorry, but I do not understand this change.  How was this tested?

thanks,

greg k-h

[PATCH v2] greybus/usb: handle unspecified lengths in hub_control

[Jose A. Perez de Azpillaga]

Fixes the FIXME in hub_control where response length was not handled
correctly.

The previous implementation always added wLength to the expected
response size, even when wLength was zero. The code also copied wLength
bytes from the response buffer without validating the actual payload
size returned by the Greybus operation.

Compute the response size starting from the fixed header and only add
wLength when it is non-zero. When copying data back to the caller, clamp
the copy size to the actual payload length reported by the Greybus core.
This avoids copying more data than what was actually returned by the
Greybus operation.

Tested by building the driver and issuing hub control requests with
varying wLength values (including zero) and verifying correct behavior.

Signed-off-by: Jose A. Perez de Azpillaga <azpijr@gmail.com>

---

Changes in v2:
- Document behavior when wLength == 0
- Clamp memcpy() size to actual payload length
- Add testing notes
---
 drivers/staging/greybus/usb.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/greybus/usb.c b/drivers/staging/greybus/usb.c
index 475f24f20cd4..f5f5a4863ddc 100644
--- a/drivers/staging/greybus/usb.c
+++ b/drivers/staging/greybus/usb.c
@@ -105,8 +105,10 @@ static int hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, u16 wIndex,
 	size_t response_size;
 	int ret;
 
-	/* FIXME: handle unspecified lengths */
-	response_size = sizeof(*response) + wLength;
+	/* Calculate expected response size */
+	response_size = sizeof(*response);
+	if (wLength)
+		response_size += wLength;
 
 	operation = gb_operation_create(dev->connection,
 					GB_USB_TYPE_HUB_CONTROL,
@@ -127,9 +129,13 @@ static int hub_control(struct usb_hcd *hcd, u16 typeReq, u16 wValue, u16 wIndex,
 		goto out;
 
 	if (wLength) {
-		/* Greybus core has verified response size */
-		response = operation->response->payload;
-		memcpy(buf, response->buf, wLength);
+		size_t actual_size = operation->response->payload_size - sizeof(*response);
+		size_t copy_size = min(wLength, actual_size);
+
+		if (copy_size) {
+			response = operation->response->payload;
+			memcpy(buf, response->buf, copy_size);
+		}
 	}
 out:
 	gb_operation_put(operation);
-- 
2.53.0