Re: [PATCH v6 3/3] usb: gadget: gadgetfs: avoid writing through the const setup pointer

[Alan Stern]

On Mon, Nov 09, 2026 at 02:50:53PM +0800, Jing Lin via B4 Relay wrote:
> From: Jing Lin <kayano050408@xxxxxxxxx>
> 
> gadgetfs_setup() receives the setup packet as const, but clamps an
> oversized control IN request by casting away the constness and writing
> the truncated length back into the caller's wLength field.
> 
> The setup() callback has no business modifying the setup packet.  Use a
> local copy of the request for the clamped length instead.  Userspace
> still observes the truncated wLength in the GADGETFS_SETUP event, and
> the queued request length is unchanged.
> 
> Suggested-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Jing Lin <kayano050408@xxxxxxxxx>
> ---
>  drivers/usb/gadget/legacy/inode.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/usb/gadget/legacy/inode.c b/drivers/usb/gadget/legacy/inode.c
> index d8dfc1897..7c48214ba 100644
> --- a/drivers/usb/gadget/legacy/inode.c
> +++ b/drivers/usb/gadget/legacy/inode.c
> @@ -1543,6 +1543,7 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
>  	struct usb_request		*req = dev->req;
>  	int				value = -EOPNOTSUPP;
>  	struct usb_gadgetfs_event	*event;
> +	struct usb_ctrlrequest		ctrl_buf;
>  	u16				w_value = le16_to_cpu(ctrl->wValue);
>  	u16				w_length = le16_to_cpu(ctrl->wLength);
>  
> @@ -1552,10 +1553,10 @@ gadgetfs_setup (struct usb_gadget *gadget, const struct usb_ctrlrequest *ctrl)
>  
>  	if (w_length > RBUF_SIZE) {
>  		if (ctrl->bRequestType & USB_DIR_IN) {
> -			/* Cast away the const, we are going to overwrite on purpose. */
> -			__le16 *temp = (__le16 *)&ctrl->wLength;
> -
> -			*temp = cpu_to_le16(RBUF_SIZE);
> +			/* Report the length we can actually handle to userspace. */
> +			ctrl_buf = *ctrl;
> +			ctrl_buf.wLength = cpu_to_le16(RBUF_SIZE);
> +			ctrl = &ctrl_buf;
>  			w_length = RBUF_SIZE;
>  		} else {
>  			spin_unlock(&dev->lock);
> 

This is kind of silly.  There's no need to copy the entire structure.

In fact, ctrl->wLength isn't used again anywhere in this routine except 
for the line that initializes event->u.setup by copying *ctrl into it.  
All you need to do is this:

 			event->u.setup = *ctrl;
+			/* Report the length we can actually handle */
+			event->u.setup.wLength = cpu_to_le16(w_length);

Alan Stern