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

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

[Taegu Ha]

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>
---
 drivers/usb/gadget/function/f_uac1_legacy.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/usb/gadget/function/f_uac1_legacy.c b/drivers/usb/gadget/function/f_uac1_legacy.c
index a0c953a99727..6d6fe5db99f5 100644
--- a/drivers/usb/gadget/function/f_uac1_legacy.c
+++ b/drivers/usb/gadget/function/f_uac1_legacy.c
@@ -11,6 +11,7 @@
 #include <linux/module.h>
 #include <linux/device.h>
 #include <linux/atomic.h>
+#include <asm/unaligned.h>
 
 #include "u_uac1_legacy.h"
 
@@ -370,9 +371,21 @@ static void f_audio_complete(struct usb_ep *ep, struct usb_request *req)
 		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));
+			struct usb_audio_control *con = audio->set_con;
+
+			if ((con->type == UAC_FU_MUTE && req->actual != sizeof(u8)) ||
+				(con->type == UAC_FU_VOLUME && req->actual != sizeof(__le16)) ||
+				(con->type != UAC_FU_MUTE && con->type != UAC_FU_VOLUME)) {
+				usb_ep_set_halt(ep);
+				audio->set_con = NULL;
+				break;
+			}
+
+			if (con->type == UAC_FU_MUTE)
+				data = *(u8 *)req->buf;
+			else
+				data = get_unaligned_le16(req->buf);
+			con->set(con, audio->set_cmd, data);
 			audio->set_con = NULL;
 		}
 		break;
-- 
2.43.0

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

[Greg KH]

On Wed, Apr 01, 2026 at 07:46:11PM +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>
> ---
>  drivers/usb/gadget/function/f_uac1_legacy.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/usb/gadget/function/f_uac1_legacy.c b/drivers/usb/gadget/function/f_uac1_legacy.c
> index a0c953a99727..6d6fe5db99f5 100644
> --- a/drivers/usb/gadget/function/f_uac1_legacy.c
> +++ b/drivers/usb/gadget/function/f_uac1_legacy.c
> @@ -11,6 +11,7 @@
>  #include <linux/module.h>
>  #include <linux/device.h>
>  #include <linux/atomic.h>
> +#include <asm/unaligned.h>
>  
>  #include "u_uac1_legacy.h"
>  
> @@ -370,9 +371,21 @@ static void f_audio_complete(struct usb_ep *ep, struct usb_request *req)
>  		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));
> +			struct usb_audio_control *con = audio->set_con;
> +
> +			if ((con->type == UAC_FU_MUTE && req->actual != sizeof(u8)) ||
> +				(con->type == UAC_FU_VOLUME && req->actual != sizeof(__le16)) ||
> +				(con->type != UAC_FU_MUTE && con->type != UAC_FU_VOLUME)) {
> +				usb_ep_set_halt(ep);
> +				audio->set_con = NULL;
> +				break;
> +			}
> +
> +			if (con->type == UAC_FU_MUTE)
> +				data = *(u8 *)req->buf;
> +			else
> +				data = get_unaligned_le16(req->buf);

Very cool, thanks for finding and fixing this.  But there is a bit of a
coding style issue in that big if check, AND it's hard to follow, how
about doing something like this instead (totally untested and not even
compiled:
			struct usb_audio_control *con = audio->set_con;
			u8 type = con->type;
			bool valid_request = false;

			switch (type) {
			case: UAC_FU_MUTE:
				if (req->actual == sizeof(__u8)) {
					memcpy(&data, req->buf, sizeof(__u8));
					valid_request = true;
				}
				break;
			case UAC_FU_VOLUME:
				if (req->actual == sizeof(__le16) {
					memcpy(&data, req->buf, sizeof(__le16));
					valid_request = true;
				}
				break;
			}
			if (valid_request)
				con->set(con, audio->set_cmd, data);
			else
				usb_ep_set_halt(ep);

  			audio->set_con = NULL;
		}

Does that make it easier to read?  I don't know, maybe not, your call,
but this way the memcpy of a constant size should be equalivant to your
direct cast if the compiler is sane, right?

And as this is a __le16 value, should we be doing some endian conversion
somewhere?

thanks,

greg k-h

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

[Taegu Ha]

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

---
 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;
+			bool valid_request = false;
+
+			switch (type) {
+			case UAC_FU_MUTE: {
+				u8 value;
+
+				if (req->actual == sizeof(value)) {
+					memcpy(&value, req->buf, sizeof(value));
+					data = value;
+					valid_request = true;
+				}
+				break;
+			}
+			case UAC_FU_VOLUME: {
+				__le16 value;
+
+				if (req->actual == sizeof(value)) {
+					memcpy(&value, req->buf, sizeof(value));
+					data = le16_to_cpu(value);
+					valid_request = true;
+				}
+				break;
+			}
+			}
+
+			if (valid_request)
+				con->set(con, audio->set_cmd, data);
+			else
+				usb_ep_set_halt(ep);
+
 			audio->set_con = NULL;
 		}
 		break;
-- 
2.43.0

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

[Greg KH]

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

[PATCH v3] usb: gadget: f_uac1_legacy: validate control request size

[Taegu Ha]

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 v3:
- drop the unnecessary zero-initialization of data
- build-test the updated change
- add KUnit coverage for valid and malformed mute/volume payload sizes

Build-tested: make drivers/usb/gadget/function/f_uac1_legacy.o
Runtime-tested: KUnit on x86_64 with KASAN
- malformed mute/volume payload sizes are rejected
- valid 1-byte mute and 2-byte volume payloads still reach the setter

---
 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..5d201a2e30e7 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;
+			bool valid_request = false;
+
+			switch (type) {
+			case UAC_FU_MUTE: {
+				u8 value;
+
+				if (req->actual == sizeof(value)) {
+					memcpy(&value, req->buf, sizeof(value));
+					data = value;
+					valid_request = true;
+				}
+				break;
+			}
+			case UAC_FU_VOLUME: {
+				__le16 value;
+
+				if (req->actual == sizeof(value)) {
+					memcpy(&value, req->buf, sizeof(value));
+					data = le16_to_cpu(value);
+					valid_request = true;
+				}
+				break;
+			}
+			}
+
+			if (valid_request)
+				con->set(con, audio->set_cmd, data);
+			else
+				usb_ep_set_halt(ep);
+
 			audio->set_con = NULL;
 		}
 		break;
-- 
2.43.0

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

[kernel test robot]

Hi Taegu,

kernel test robot noticed the following build errors:

[auto build test ERROR on westeri-thunderbolt/next]
[cannot apply to usb/usb-testing usb/usb-next usb/usb-linus linus/master v7.0-rc7 next-20260410]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Taegu-Ha/usb-gadget-f_uac1_legacy-validate-control-request-size/20260412-113035
base:   https://git.kernel.org/pub/scm/linux/kernel/git/westeri/thunderbolt.git next
patch link:    https://lore.kernel.org/r/20260401104611.3375330-1-hataegu0826%40gmail.com
patch subject: [PATCH] usb: gadget: f_uac1_legacy: validate control request size
config: sh-allyesconfig (https://download.01.org/0day-ci/archive/20260412/202604121754.9JyQH6bx-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260412/202604121754.9JyQH6bx-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202604121754.9JyQH6bx-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/usb/gadget/function/f_uac1_legacy.c:14:10: fatal error: asm/unaligned.h: No such file or directory
      14 | #include <asm/unaligned.h>
         |          ^~~~~~~~~~~~~~~~~
   compilation terminated.


vim +14 drivers/usb/gadget/function/f_uac1_legacy.c

  > 14	#include <asm/unaligned.h>
    15	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki