Re: [PATCH v2] usb: gadget: f_uac1_legacy: validate control request size

[Greg KH <gregkh@linuxfoundation.org>]

On Thu, Apr 02, 2026 at 12:15:39AM +0900, Taegu Ha wrote:
> f_audio_complete() copies req->length bytes into a 4-byte stack
> variable:
> 
>   u32 data = 0;
>   memcpy(&data, req->buf, req->length);
> 
> req->length is derived from the host-controlled USB request path,
> which can lead to a stack out-of-bounds write.
> 
> Validate req->actual against the expected payload size for the
> supported control selectors and decode only the expected amount
> of data.
> 
> This avoids copying a host-influenced length into a fixed-size
> stack object.
> 
> Signed-off-by: Taegu Ha <hataegu0826@gmail.com>
> 
> Changes in v2:
> - rewrite the validation logic into a switch on control type
> - use constant-size memcpy() for fixed-size payloads
> - convert the volume payload with le16_to_cpu()
> 
> Build-tested: not tested, build environment not prepared

As you found and tested your previous change, can you please test this
one to verify it solves the issue?



> 
> ---
>  drivers/usb/gadget/function/f_uac1_legacy.c | 47 ++++++++++++++++-----
>  1 file changed, 37 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_uac1_legacy.c b/drivers/usb/gadget/function/f_uac1_legacy.c
> index a0c953a99727..00cc7161db66 100644
> --- a/drivers/usb/gadget/function/f_uac1_legacy.c
> +++ b/drivers/usb/gadget/function/f_uac1_legacy.c
> @@ -360,19 +360,46 @@ static int f_audio_out_ep_complete(struct usb_ep *ep, struct usb_request *req)
>  static void f_audio_complete(struct usb_ep *ep, struct usb_request *req)
>  {
>  	struct f_audio *audio = req->context;
> -	int status = req->status;
> -	u32 data = 0;
>  	struct usb_ep *out_ep = audio->out_ep;
>  
> -	switch (status) {
> -
> -	case 0:				/* normal completion? */
> -		if (ep == out_ep)
> +	switch (req->status) {
> +	case 0:
> +		if (ep == out_ep) {
>  			f_audio_out_ep_complete(ep, req);
> -		else if (audio->set_con) {
> -			memcpy(&data, req->buf, req->length);
> -			audio->set_con->set(audio->set_con, audio->set_cmd,
> -					le16_to_cpu(data));
> +		} else if (audio->set_con) {
> +			struct usb_audio_control *con = audio->set_con;
> +			u8 type = con->type;
> +			u32 data = 0;

No need to set this to 0 ahead of time, right?

thanks,

greg k-h