Linux USB
 help / color / mirror / Atom feed
* [PATCH] usb: gadget: f_uvc: fix Extension Unit descriptor heap overflow
@ 2026-08-22 19:47 Haofeng Li
  2026-08-22 20:01 ` Randy Dunlap
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Haofeng Li @ 2026-08-22 19:47 UTC (permalink / raw)
  To: gregkh, xu.yang_2
  Cc: hhhuuu, kees, kai.aizen.dev, andriy.shevchenko, rdunlap,
	christophe.jaillet, dan.scally, linux-usb, linux-kernel,
	13266079573, Haofeng Li

An Extension Unit descriptor is 24 + bNrInPins + bControlSize bytes long
(UVC_DT_EXTENSION_UNIT_SIZE(p, n)), where bNrInPins and bControlSize are
configfs attributes each accepted over the full 0..255 range by
kstrtou8().  uvc_configfs stores the computed size in the u8 bLength
field of the descriptor, so once 24 + p + n exceeds 255 it silently
wraps: p = n = 255 describes a 534-byte descriptor with bLength = 22.

uvc_copy_descriptors() reserves xu->desc.bLength bytes per Extension Unit
in the descriptor buffer it allocates at bind time, but
UVC_COPY_XU_DESCRIPTOR() copies the real descriptor contents, i.e.
22 + bNrInPins + 1 + bControlSize + 1 bytes.  With the wrapped length the
copy overruns the allocation by up to 512 bytes.

Attack chain (write access to a UVC gadget's configfs attributes; no
race, no USB traffic, a single bind triggers it):

    echo 255 > .../functions/uvc.0/extensions/ext.0/b_nr_in_pins
    echo 255 > .../functions/uvc.0/extensions/ext.0/b_control_size
      -> uvcg_extension_b_nr_in_pins_store() /
         uvcg_extension_b_control_size_store()
      -> bLength = UVC_DT_EXTENSION_UNIT_SIZE(255, 255) wraps to 22
    bind the gadget to a UDC
      -> uvc_function_bind() -> uvc_copy_descriptors()
      -> kmalloc() sized using the wrapped bLength: 22 bytes for the XU
      -> UVC_COPY_XU_DESCRIPTOR() writes the real 534 bytes into that
         slot (22-byte head, 255 baSourceID, bControlSize, 255
         bmControls, iExtension)
      -> heap out-of-bounds write during bind

Reproduced on 7.2.0+: a stock build (FORTIFY on) can derive the remaining
allocation size at the bmControls memcpy and BUGs in __fortify_panic()
during bind - a deterministic kernel crash; with FORTIFY disabled for the
file, KASAN reports "slab-out-of-bounds Write of size 255" in
uvc_copy_descriptors() against the kmalloc-192 descriptor buffer.

Reject combinations whose descriptor does not fit into bLength at all four
configfs entry points that can grow an Extension Unit (b_nr_in_pins,
b_control_size, ba_source_id, bm_controls), and make uvc_copy_descriptors()
refuse an Extension Unit whose bLength does not match its contents instead
of overflowing the buffer.

Fixes: 0525210c9840 ("usb: gadget: uvc: Allow definition of XUs in configfs")
Signed-off-by: Haofeng Li <lihaofeng@kylinos.cn>
Assisted-by: opencode:deepseek-v4-flash-free
---
 drivers/usb/gadget/function/f_uvc.c        |  6 ++++++
 drivers/usb/gadget/function/uvc_configfs.c | 25 ++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 73dc7e42875f..0d3432f80cae 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -598,6 +598,12 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed)
 	}
 
 	list_for_each_entry(xu, uvc->desc.extension_units, list) {
+		/* Mismatched bLength would overflow the buffer sized after it */
+		if (xu->desc.bLength !=
+		    UVC_DT_EXTENSION_UNIT_SIZE(xu->desc.bNrInPins,
+					       xu->desc.bControlSize))
+			return ERR_PTR(-EINVAL);
+
 		control_size += xu->desc.bLength;
 		bytes += xu->desc.bLength;
 		n_desc++;
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 70a1415ea401..a584a6b85b06 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -845,6 +845,15 @@ static ssize_t uvcg_extension_b_num_controls_store(struct config_item *item,
 }
 UVCG_EXTENSION_ATTR(b_num_controls, bNumControls);
 
+/* The descriptor must fit into the one-byte bLength field */
+static int uvcg_extension_check_size(u8 nr_in_pins, u8 control_size)
+{
+	if (UVC_DT_EXTENSION_UNIT_SIZE(nr_in_pins, control_size) > 255)
+		return -EINVAL;
+
+	return 0;
+}
+
 /*
  * In addition to storing bNrInPins, this function needs to realloc the
  * memory for the baSourceID array and additionally expand bLength.
@@ -877,6 +886,10 @@ static ssize_t uvcg_extension_b_nr_in_pins_store(struct config_item *item,
 		goto unlock;
 	}
 
+	ret = uvcg_extension_check_size(num, xu->desc.bControlSize);
+	if (ret)
+		goto unlock;
+
 	tmp_buf = krealloc_array(xu->desc.baSourceID, num, sizeof(u8),
 				 GFP_KERNEL | __GFP_ZERO);
 	if (!tmp_buf) {
@@ -930,6 +943,10 @@ static ssize_t uvcg_extension_b_control_size_store(struct config_item *item,
 		goto unlock;
 	}
 
+	ret = uvcg_extension_check_size(xu->desc.bNrInPins, num);
+	if (ret)
+		goto unlock;
+
 	tmp_buf = krealloc_array(xu->desc.bmControls, num, sizeof(u8),
 				 GFP_KERNEL | __GFP_ZERO);
 	if (!tmp_buf) {
@@ -1055,6 +1072,10 @@ static ssize_t uvcg_extension_ba_source_id_store(struct config_item *item,
 	if (ret)
 		goto unlock;
 
+	ret = uvcg_extension_check_size(n, xu->desc.bControlSize);
+	if (ret)
+		goto unlock;
+
 	iter = source_ids = kcalloc(n, sizeof(u8), GFP_KERNEL);
 	if (!source_ids) {
 		ret = -ENOMEM;
@@ -1134,6 +1155,10 @@ static ssize_t uvcg_extension_bm_controls_store(struct config_item *item,
 	if (ret)
 		goto unlock;
 
+	ret = uvcg_extension_check_size(xu->desc.bNrInPins, n);
+	if (ret)
+		goto unlock;
+
 	iter = bm_controls = kcalloc(n, sizeof(u8), GFP_KERNEL);
 	if (!bm_controls) {
 		ret = -ENOMEM;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] usb: gadget: f_uvc: fix Extension Unit descriptor heap overflow
  2026-08-22 19:47 [PATCH] usb: gadget: f_uvc: fix Extension Unit descriptor heap overflow Haofeng Li
@ 2026-08-22 20:01 ` Randy Dunlap
  2026-08-22 20:26   ` Haofeng Li
  2026-08-22 20:26 ` [PATCH v2] " Haofeng Li
  2026-08-24  8:38 ` [PATCH] " Dan Scally
  2 siblings, 1 reply; 8+ messages in thread
From: Randy Dunlap @ 2026-08-22 20:01 UTC (permalink / raw)
  To: Haofeng Li, gregkh, xu.yang_2
  Cc: hhhuuu, kees, kai.aizen.dev, andriy.shevchenko,
	christophe.jaillet, dan.scally, linux-usb, linux-kernel,
	13266079573



On 8/22/26 12:47 PM, Haofeng Li wrote:
> +/* The descriptor must fit into the one-byte bLength field */

          descriptor length must fit
?

> +static int uvcg_extension_check_size(u8 nr_in_pins, u8 control_size)
> +{
> +	if (UVC_DT_EXTENSION_UNIT_SIZE(nr_in_pins, control_size) > 255)
> +		return -EINVAL;
> +
> +	return 0;
> +}

-- 
~Randy


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] usb: gadget: f_uvc: fix Extension Unit descriptor heap overflow
  2026-08-22 20:01 ` Randy Dunlap
@ 2026-08-22 20:26   ` Haofeng Li
  0 siblings, 0 replies; 8+ messages in thread
From: Haofeng Li @ 2026-08-22 20:26 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: gregkh, xu.yang_2, hhhuuu, kees, kai.aizen.dev, andriy.shevchenko,
	christophe.jaillet, dan.scally, linux-usb, linux-kernel,
	13266079573, Haofeng Li

On 8/22/26 13:01, Randy Dunlap wrote:
>> +/* The descriptor must fit into the one-byte bLength field */
>
>          descriptor length must fit
> ?

Good point, thanks. Fixed in v2:

	-/* The descriptor must fit into the one-byte bLength field */
	+/* The descriptor length must fit into the one-byte bLength field */

v2 sent as a reply to the original patch.

Regards,
Haofeng

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2] usb: gadget: f_uvc: fix Extension Unit descriptor heap overflow
  2026-08-22 19:47 [PATCH] usb: gadget: f_uvc: fix Extension Unit descriptor heap overflow Haofeng Li
  2026-08-22 20:01 ` Randy Dunlap
@ 2026-08-22 20:26 ` Haofeng Li
  2026-08-24  8:38 ` [PATCH] " Dan Scally
  2 siblings, 0 replies; 8+ messages in thread
From: Haofeng Li @ 2026-08-22 20:26 UTC (permalink / raw)
  To: linux-usb
  Cc: gregkh, xu.yang_2, Randy Dunlap, hhhuuu, kees, kai.aizen.dev,
	andriy.shevchenko, christophe.jaillet, dan.scally, linux-kernel,
	13266079573, Haofeng Li

An Extension Unit descriptor is 24 + bNrInPins + bControlSize bytes long
(UVC_DT_EXTENSION_UNIT_SIZE(p, n)), where bNrInPins and bControlSize are
configfs attributes each accepted over the full 0..255 range by
kstrtou8().  uvc_configfs stores the computed size in the u8 bLength
field of the descriptor, so once 24 + p + n exceeds 255 it silently
wraps: p = n = 255 describes a 534-byte descriptor with bLength = 22.

uvc_copy_descriptors() reserves xu->desc.bLength bytes per Extension Unit
in the descriptor buffer it allocates at bind time, but
UVC_COPY_XU_DESCRIPTOR() copies the real descriptor contents, i.e.
22 + bNrInPins + 1 + bControlSize + 1 bytes.  With the wrapped length the
copy overruns the allocation by up to 512 bytes.

Attack chain (write access to a UVC gadget's configfs attributes; no
race, no USB traffic, a single bind triggers it):

    echo 255 > .../functions/uvc.0/extensions/ext.0/b_nr_in_pins
    echo 255 > .../functions/uvc.0/extensions/ext.0/b_control_size
      -> uvcg_extension_b_nr_in_pins_store() /
         uvcg_extension_b_control_size_store()
      -> bLength = UVC_DT_EXTENSION_UNIT_SIZE(255, 255) wraps to 22
    bind the gadget to a UDC
      -> uvc_function_bind() -> uvc_copy_descriptors()
      -> kmalloc() sized using the wrapped bLength: 22 bytes for the XU
      -> UVC_COPY_XU_DESCRIPTOR() writes the real 534 bytes into that
         slot (22-byte head, 255 baSourceID, bControlSize, 255
         bmControls, iExtension)
      -> heap out-of-bounds write during bind

Reproduced on 7.2.0+: a stock build (FORTIFY on) can derive the remaining
allocation size at the bmControls memcpy and BUGs in __fortify_panic()
during bind - a deterministic kernel crash; with FORTIFY disabled for the
file, KASAN reports "slab-out-of-bounds Write of size 255" in
uvc_copy_descriptors() against the kmalloc-192 descriptor buffer.

Reject combinations whose descriptor does not fit into bLength at all four
configfs entry points that can grow an Extension Unit (b_nr_in_pins,
b_control_size, ba_source_id, bm_controls), and make uvc_copy_descriptors()
refuse an Extension Unit whose bLength does not match its contents instead
of overflowing the buffer.

Fixes: 0525210c9840 ("usb: gadget: uvc: Allow definition of XUs in configfs")
Signed-off-by: Haofeng Li <lihaofeng@kylinos.cn>
Assisted-by: opencode:deepseek-v4-flash-free
---
Changes in v2:
- reword a comment per Randy Dunlap: "The descriptor length must fit
  into the one-byte bLength field".

 drivers/usb/gadget/function/f_uvc.c        |  6 ++++++
 drivers/usb/gadget/function/uvc_configfs.c | 25 ++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 73dc7e42875f..0d3432f80cae 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -598,6 +598,12 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed)
 	}
 
 	list_for_each_entry(xu, uvc->desc.extension_units, list) {
+		/* Mismatched bLength would overflow the buffer sized after it */
+		if (xu->desc.bLength !=
+		    UVC_DT_EXTENSION_UNIT_SIZE(xu->desc.bNrInPins,
+					       xu->desc.bControlSize))
+			return ERR_PTR(-EINVAL);
+
 		control_size += xu->desc.bLength;
 		bytes += xu->desc.bLength;
 		n_desc++;
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 70a1415ea401..d5ba9c409983 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -845,6 +845,15 @@ static ssize_t uvcg_extension_b_num_controls_store(struct config_item *item,
 }
 UVCG_EXTENSION_ATTR(b_num_controls, bNumControls);
 
+/* The descriptor length must fit into the one-byte bLength field */
+static int uvcg_extension_check_size(u8 nr_in_pins, u8 control_size)
+{
+	if (UVC_DT_EXTENSION_UNIT_SIZE(nr_in_pins, control_size) > 255)
+		return -EINVAL;
+
+	return 0;
+}
+
 /*
  * In addition to storing bNrInPins, this function needs to realloc the
  * memory for the baSourceID array and additionally expand bLength.
@@ -877,6 +886,10 @@ static ssize_t uvcg_extension_b_nr_in_pins_store(struct config_item *item,
 		goto unlock;
 	}
 
+	ret = uvcg_extension_check_size(num, xu->desc.bControlSize);
+	if (ret)
+		goto unlock;
+
 	tmp_buf = krealloc_array(xu->desc.baSourceID, num, sizeof(u8),
 				 GFP_KERNEL | __GFP_ZERO);
 	if (!tmp_buf) {
@@ -930,6 +943,10 @@ static ssize_t uvcg_extension_b_control_size_store(struct config_item *item,
 		goto unlock;
 	}
 
+	ret = uvcg_extension_check_size(xu->desc.bNrInPins, num);
+	if (ret)
+		goto unlock;
+
 	tmp_buf = krealloc_array(xu->desc.bmControls, num, sizeof(u8),
 				 GFP_KERNEL | __GFP_ZERO);
 	if (!tmp_buf) {
@@ -1055,6 +1072,10 @@ static ssize_t uvcg_extension_ba_source_id_store(struct config_item *item,
 	if (ret)
 		goto unlock;
 
+	ret = uvcg_extension_check_size(n, xu->desc.bControlSize);
+	if (ret)
+		goto unlock;
+
 	iter = source_ids = kcalloc(n, sizeof(u8), GFP_KERNEL);
 	if (!source_ids) {
 		ret = -ENOMEM;
@@ -1134,6 +1155,10 @@ static ssize_t uvcg_extension_bm_controls_store(struct config_item *item,
 	if (ret)
 		goto unlock;
 
+	ret = uvcg_extension_check_size(xu->desc.bNrInPins, n);
+	if (ret)
+		goto unlock;
+
 	iter = bm_controls = kcalloc(n, sizeof(u8), GFP_KERNEL);
 	if (!bm_controls) {
 		ret = -ENOMEM;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] usb: gadget: f_uvc: fix Extension Unit descriptor heap overflow
  2026-08-22 19:47 [PATCH] usb: gadget: f_uvc: fix Extension Unit descriptor heap overflow Haofeng Li
  2026-08-22 20:01 ` Randy Dunlap
  2026-08-22 20:26 ` [PATCH v2] " Haofeng Li
@ 2026-08-24  8:38 ` Dan Scally
  2026-08-24  9:44   ` [PATCH v3] " Haofeng Li
  2 siblings, 1 reply; 8+ messages in thread
From: Dan Scally @ 2026-08-24  8:38 UTC (permalink / raw)
  To: Haofeng Li, gregkh, xu.yang_2
  Cc: hhhuuu, kees, kai.aizen.dev, andriy.shevchenko, rdunlap,
	christophe.jaillet, linux-usb, linux-kernel, 13266079573

Hi Haofeng, thanks for the patch

On 22/08/2026 20:47, Haofeng Li wrote:
> An Extension Unit descriptor is 24 + bNrInPins + bControlSize bytes long
> (UVC_DT_EXTENSION_UNIT_SIZE(p, n)), where bNrInPins and bControlSize are
> configfs attributes each accepted over the full 0..255 range by
> kstrtou8().  uvc_configfs stores the computed size in the u8 bLength
> field of the descriptor, so once 24 + p + n exceeds 255 it silently
> wraps: p = n = 255 describes a 534-byte descriptor with bLength = 22.

Ouch, thanks for catching this.

> uvc_copy_descriptors() reserves xu->desc.bLength bytes per Extension Unit
> in the descriptor buffer it allocates at bind time, but
> UVC_COPY_XU_DESCRIPTOR() copies the real descriptor contents, i.e.
> 22 + bNrInPins + 1 + bControlSize + 1 bytes.  With the wrapped length the
> copy overruns the allocation by up to 512 bytes.
> 
> Attack chain (write access to a UVC gadget's configfs attributes; no
> race, no USB traffic, a single bind triggers it):
> 
>      echo 255 > .../functions/uvc.0/extensions/ext.0/b_nr_in_pins
>      echo 255 > .../functions/uvc.0/extensions/ext.0/b_control_size
>        -> uvcg_extension_b_nr_in_pins_store() /
>           uvcg_extension_b_control_size_store()
>        -> bLength = UVC_DT_EXTENSION_UNIT_SIZE(255, 255) wraps to 22
>      bind the gadget to a UDC
>        -> uvc_function_bind() -> uvc_copy_descriptors()
>        -> kmalloc() sized using the wrapped bLength: 22 bytes for the XU
>        -> UVC_COPY_XU_DESCRIPTOR() writes the real 534 bytes into that
>           slot (22-byte head, 255 baSourceID, bControlSize, 255
>           bmControls, iExtension)
>        -> heap out-of-bounds write during bind
> 
> Reproduced on 7.2.0+: a stock build (FORTIFY on) can derive the remaining
> allocation size at the bmControls memcpy and BUGs in __fortify_panic()
> during bind - a deterministic kernel crash; with FORTIFY disabled for the
> file, KASAN reports "slab-out-of-bounds Write of size 255" in
> uvc_copy_descriptors() against the kmalloc-192 descriptor buffer.
> 
> Reject combinations whose descriptor does not fit into bLength at all four
> configfs entry points that can grow an Extension Unit (b_nr_in_pins,
> b_control_size, ba_source_id, bm_controls), and make uvc_copy_descriptors()
> refuse an Extension Unit whose bLength does not match its contents instead
> of overflowing the buffer.
> 
> Fixes: 0525210c9840 ("usb: gadget: uvc: Allow definition of XUs in configfs")
> Signed-off-by: Haofeng Li <lihaofeng@kylinos.cn>
> Assisted-by: opencode:deepseek-v4-flash-free
> ---
>   drivers/usb/gadget/function/f_uvc.c        |  6 ++++++
>   drivers/usb/gadget/function/uvc_configfs.c | 25 ++++++++++++++++++++++
>   2 files changed, 31 insertions(+)
> 
> diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
> index 73dc7e42875f..0d3432f80cae 100644
> --- a/drivers/usb/gadget/function/f_uvc.c
> +++ b/drivers/usb/gadget/function/f_uvc.c
> @@ -598,6 +598,12 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed)
>   	}
>   
>   	list_for_each_entry(xu, uvc->desc.extension_units, list) {
> +		/* Mismatched bLength would overflow the buffer sized after it */
> +		if (xu->desc.bLength !=
> +		    UVC_DT_EXTENSION_UNIT_SIZE(xu->desc.bNrInPins,
> +					       xu->desc.bControlSize))
> +			return ERR_PTR(-EINVAL);
> +
>   		control_size += xu->desc.bLength;
>   		bytes += xu->desc.bLength;
>   		n_desc++;
> diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
> index 70a1415ea401..a584a6b85b06 100644
> --- a/drivers/usb/gadget/function/uvc_configfs.c
> +++ b/drivers/usb/gadget/function/uvc_configfs.c
> @@ -845,6 +845,15 @@ static ssize_t uvcg_extension_b_num_controls_store(struct config_item *item,
>   }
>   UVCG_EXTENSION_ATTR(b_num_controls, bNumControls);
>   
> +/* The descriptor must fit into the one-byte bLength field */
> +static int uvcg_extension_check_size(u8 nr_in_pins, u8 control_size)
> +{
> +	if (UVC_DT_EXTENSION_UNIT_SIZE(nr_in_pins, control_size) > 255)
> +		return -EINVAL;
> +
> +	return 0;
> +}

I think my one comment on the patch is that a warning message would be worthwhile here, since the 
error can arise from user input to configfs. With that:

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

Thanks
Dan

> +
>   /*
>    * In addition to storing bNrInPins, this function needs to realloc the
>    * memory for the baSourceID array and additionally expand bLength.
> @@ -877,6 +886,10 @@ static ssize_t uvcg_extension_b_nr_in_pins_store(struct config_item *item,
>   		goto unlock;
>   	}
>   
> +	ret = uvcg_extension_check_size(num, xu->desc.bControlSize);
> +	if (ret)
> +		goto unlock;
> +
>   	tmp_buf = krealloc_array(xu->desc.baSourceID, num, sizeof(u8),
>   				 GFP_KERNEL | __GFP_ZERO);
>   	if (!tmp_buf) {
> @@ -930,6 +943,10 @@ static ssize_t uvcg_extension_b_control_size_store(struct config_item *item,
>   		goto unlock;
>   	}
>   
> +	ret = uvcg_extension_check_size(xu->desc.bNrInPins, num);
> +	if (ret)
> +		goto unlock;
> +
>   	tmp_buf = krealloc_array(xu->desc.bmControls, num, sizeof(u8),
>   				 GFP_KERNEL | __GFP_ZERO);
>   	if (!tmp_buf) {
> @@ -1055,6 +1072,10 @@ static ssize_t uvcg_extension_ba_source_id_store(struct config_item *item,
>   	if (ret)
>   		goto unlock;
>   
> +	ret = uvcg_extension_check_size(n, xu->desc.bControlSize);
> +	if (ret)
> +		goto unlock;
> +
>   	iter = source_ids = kcalloc(n, sizeof(u8), GFP_KERNEL);
>   	if (!source_ids) {
>   		ret = -ENOMEM;
> @@ -1134,6 +1155,10 @@ static ssize_t uvcg_extension_bm_controls_store(struct config_item *item,
>   	if (ret)
>   		goto unlock;
>   
> +	ret = uvcg_extension_check_size(xu->desc.bNrInPins, n);
> +	if (ret)
> +		goto unlock;
> +
>   	iter = bm_controls = kcalloc(n, sizeof(u8), GFP_KERNEL);
>   	if (!bm_controls) {
>   		ret = -ENOMEM;


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v3] usb: gadget: f_uvc: fix Extension Unit descriptor heap overflow
  2026-08-24  8:38 ` [PATCH] " Dan Scally
@ 2026-08-24  9:44   ` Haofeng Li
  2026-08-24 15:29     ` Andy Shevchenko
  2026-08-24 17:04     ` [PATCH v4] " Haofeng Li
  0 siblings, 2 replies; 8+ messages in thread
From: Haofeng Li @ 2026-08-24  9:44 UTC (permalink / raw)
  To: dan.scally, gregkh, xu.yang_2
  Cc: lihaofeng, hhhuuu, kees, kai.aizen.dev, andriy.shevchenko,
	rdunlap, christophe.jaillet, linux-usb, linux-kernel, 13266079573

An Extension Unit descriptor is 24 + bNrInPins + bControlSize bytes long
(UVC_DT_EXTENSION_UNIT_SIZE(p, n)), where bNrInPins and bControlSize are
configfs attributes each accepted over the full 0..255 range by
kstrtou8().  uvc_configfs stores the computed size in the u8 bLength
field of the descriptor, so once 24 + p + n exceeds 255 it silently
wraps: p = n = 255 describes a 534-byte descriptor with bLength = 22.

uvc_copy_descriptors() reserves xu->desc.bLength bytes per Extension Unit
in the descriptor buffer it allocates at bind time, but
UVC_COPY_XU_DESCRIPTOR() copies the real descriptor contents, i.e.
22 + bNrInPins + 1 + bControlSize + 1 bytes.  With the wrapped length the
copy overruns the allocation by up to 512 bytes.

Attack chain (write access to a UVC gadget's configfs attributes; no
race, no USB traffic, a single bind triggers it):

    echo 255 > .../functions/uvc.0/extensions/ext.0/b_nr_in_pins
    echo 255 > .../functions/uvc.0/extensions/ext.0/b_control_size
      -> uvcg_extension_b_nr_in_pins_store() /
         uvcg_extension_b_control_size_store()
      -> bLength = UVC_DT_EXTENSION_UNIT_SIZE(255, 255) wraps to 22
    bind the gadget to a UDC
      -> uvc_function_bind() -> uvc_copy_descriptors()
      -> kmalloc() sized using the wrapped bLength: 22 bytes for the XU
      -> UVC_COPY_XU_DESCRIPTOR() writes the real 534 bytes into that
         slot (22-byte head, 255 baSourceID, bControlSize, 255
         bmControls, iExtension)
      -> heap out-of-bounds write during bind

Reproduced on 7.2.0+: a stock build (FORTIFY on) can derive the remaining
allocation size at the bmControls memcpy and BUGs in __fortify_panic()
during bind - a deterministic kernel crash; with FORTIFY disabled for the
file, KASAN reports "slab-out-of-bounds Write of size 255" in
uvc_copy_descriptors() against the kmalloc-192 descriptor buffer.

Reject combinations whose descriptor does not fit into bLength at all four
configfs entry points that can grow an Extension Unit (b_nr_in_pins,
b_control_size, ba_source_id, bm_controls), and make uvc_copy_descriptors()
refuse an Extension Unit whose bLength does not match its contents instead
of overflowing the buffer.

Fixes: 0525210c9840 ("usb: gadget: uvc: Allow definition of XUs in configfs")
Signed-off-by: Haofeng Li <lihaofeng@kylinos.cn>
Assisted-by: opencode:deepseek-v4-flash-free

Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
---
Changes in v2:
- reword a comment per Randy Dunlap: "The descriptor length must fit
  into the one-byte bLength field".

Changes in v3:
- Add a warning message when the descriptor size is rejected, as suggested
  by Dan Scally.
 drivers/usb/gadget/function/f_uvc.c        |  6 +++++
 drivers/usb/gadget/function/uvc_configfs.c | 28 ++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 73dc7e42875f..0d3432f80cae 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -598,6 +598,12 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed)
 	}
 
 	list_for_each_entry(xu, uvc->desc.extension_units, list) {
+		/* Mismatched bLength would overflow the buffer sized after it */
+		if (xu->desc.bLength !=
+		    UVC_DT_EXTENSION_UNIT_SIZE(xu->desc.bNrInPins,
+					       xu->desc.bControlSize))
+			return ERR_PTR(-EINVAL);
+
 		control_size += xu->desc.bLength;
 		bytes += xu->desc.bLength;
 		n_desc++;
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 70a1415ea401..b52c9cf2b715 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -845,6 +845,18 @@ static ssize_t uvcg_extension_b_num_controls_store(struct config_item *item,
 }
 UVCG_EXTENSION_ATTR(b_num_controls, bNumControls);
 
+/* The descriptor length must fit into the one-byte bLength field */
+static int uvcg_extension_check_size(u8 nr_in_pins, u8 control_size)
+{
+	if (UVC_DT_EXTENSION_UNIT_SIZE(nr_in_pins, control_size) > 255) {
+		pr_warn("uvc: Extension Unit descriptor size (%u + %u) does not fit into bLength\n",
+			nr_in_pins, control_size);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 /*
  * In addition to storing bNrInPins, this function needs to realloc the
  * memory for the baSourceID array and additionally expand bLength.
@@ -877,6 +889,10 @@ static ssize_t uvcg_extension_b_nr_in_pins_store(struct config_item *item,
 		goto unlock;
 	}
 
+	ret = uvcg_extension_check_size(num, xu->desc.bControlSize);
+	if (ret)
+		goto unlock;
+
 	tmp_buf = krealloc_array(xu->desc.baSourceID, num, sizeof(u8),
 				 GFP_KERNEL | __GFP_ZERO);
 	if (!tmp_buf) {
@@ -930,6 +946,10 @@ static ssize_t uvcg_extension_b_control_size_store(struct config_item *item,
 		goto unlock;
 	}
 
+	ret = uvcg_extension_check_size(xu->desc.bNrInPins, num);
+	if (ret)
+		goto unlock;
+
 	tmp_buf = krealloc_array(xu->desc.bmControls, num, sizeof(u8),
 				 GFP_KERNEL | __GFP_ZERO);
 	if (!tmp_buf) {
@@ -1055,6 +1075,10 @@ static ssize_t uvcg_extension_ba_source_id_store(struct config_item *item,
 	if (ret)
 		goto unlock;
 
+	ret = uvcg_extension_check_size(n, xu->desc.bControlSize);
+	if (ret)
+		goto unlock;
+
 	iter = source_ids = kcalloc(n, sizeof(u8), GFP_KERNEL);
 	if (!source_ids) {
 		ret = -ENOMEM;
@@ -1134,6 +1158,10 @@ static ssize_t uvcg_extension_bm_controls_store(struct config_item *item,
 	if (ret)
 		goto unlock;
 
+	ret = uvcg_extension_check_size(xu->desc.bNrInPins, n);
+	if (ret)
+		goto unlock;
+
 	iter = bm_controls = kcalloc(n, sizeof(u8), GFP_KERNEL);
 	if (!bm_controls) {
 		ret = -ENOMEM;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v3] usb: gadget: f_uvc: fix Extension Unit descriptor heap overflow
  2026-08-24  9:44   ` [PATCH v3] " Haofeng Li
@ 2026-08-24 15:29     ` Andy Shevchenko
  2026-08-24 17:04     ` [PATCH v4] " Haofeng Li
  1 sibling, 0 replies; 8+ messages in thread
From: Andy Shevchenko @ 2026-08-24 15:29 UTC (permalink / raw)
  To: Haofeng Li
  Cc: dan.scally, gregkh, xu.yang_2, hhhuuu, kees, kai.aizen.dev,
	rdunlap, christophe.jaillet, linux-usb, linux-kernel, 13266079573

On Mon, Aug 24, 2026 at 05:44:56PM +0800, Haofeng Li wrote:
> An Extension Unit descriptor is 24 + bNrInPins + bControlSize bytes long
> (UVC_DT_EXTENSION_UNIT_SIZE(p, n)), where bNrInPins and bControlSize are
> configfs attributes each accepted over the full 0..255 range by
> kstrtou8().  uvc_configfs stores the computed size in the u8 bLength
> field of the descriptor, so once 24 + p + n exceeds 255 it silently
> wraps: p = n = 255 describes a 534-byte descriptor with bLength = 22.
> 
> uvc_copy_descriptors() reserves xu->desc.bLength bytes per Extension Unit
> in the descriptor buffer it allocates at bind time, but
> UVC_COPY_XU_DESCRIPTOR() copies the real descriptor contents, i.e.
> 22 + bNrInPins + 1 + bControlSize + 1 bytes.  With the wrapped length the
> copy overruns the allocation by up to 512 bytes.
> 
> Attack chain (write access to a UVC gadget's configfs attributes; no
> race, no USB traffic, a single bind triggers it):
> 
>     echo 255 > .../functions/uvc.0/extensions/ext.0/b_nr_in_pins
>     echo 255 > .../functions/uvc.0/extensions/ext.0/b_control_size
>       -> uvcg_extension_b_nr_in_pins_store() /
>          uvcg_extension_b_control_size_store()
>       -> bLength = UVC_DT_EXTENSION_UNIT_SIZE(255, 255) wraps to 22
>     bind the gadget to a UDC
>       -> uvc_function_bind() -> uvc_copy_descriptors()
>       -> kmalloc() sized using the wrapped bLength: 22 bytes for the XU
>       -> UVC_COPY_XU_DESCRIPTOR() writes the real 534 bytes into that
>          slot (22-byte head, 255 baSourceID, bControlSize, 255
>          bmControls, iExtension)
>       -> heap out-of-bounds write during bind
> 
> Reproduced on 7.2.0+: a stock build (FORTIFY on) can derive the remaining
> allocation size at the bmControls memcpy and BUGs in __fortify_panic()
> during bind - a deterministic kernel crash; with FORTIFY disabled for the
> file, KASAN reports "slab-out-of-bounds Write of size 255" in
> uvc_copy_descriptors() against the kmalloc-192 descriptor buffer.
> 
> Reject combinations whose descriptor does not fit into bLength at all four
> configfs entry points that can grow an Extension Unit (b_nr_in_pins,
> b_control_size, ba_source_id, bm_controls), and make uvc_copy_descriptors()
> refuse an Extension Unit whose bLength does not match its contents instead
> of overflowing the buffer.
> 
> Fixes: 0525210c9840 ("usb: gadget: uvc: Allow definition of XUs in configfs")
> Signed-off-by: Haofeng Li <lihaofeng@kylinos.cn>
> Assisted-by: opencode:deepseek-v4-flash-free
> 
> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

Should be no blank lines in the tag block. You may use `b4` tool (check your
Linux distro for the respective package) to automate that.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v4] usb: gadget: f_uvc: fix Extension Unit descriptor heap overflow
  2026-08-24  9:44   ` [PATCH v3] " Haofeng Li
  2026-08-24 15:29     ` Andy Shevchenko
@ 2026-08-24 17:04     ` Haofeng Li
  1 sibling, 0 replies; 8+ messages in thread
From: Haofeng Li @ 2026-08-24 17:04 UTC (permalink / raw)
  To: gregkh, linux-usb
  Cc: dan.scally, andriy.shevchenko, xu.yang_2, hhhuuu, kees,
	kai.aizen.dev, rdunlap, christophe.jaillet, linux-kernel,
	13266079573, Haofeng Li

An Extension Unit descriptor is 24 + bNrInPins + bControlSize bytes long
(UVC_DT_EXTENSION_UNIT_SIZE(p, n)), where bNrInPins and bControlSize are
configfs attributes each accepted over the full 0..255 range by
kstrtou8().  uvc_configfs stores the computed size in the u8 bLength
field of the descriptor, so once 24 + p + n exceeds 255 it silently
wraps: p = n = 255 describes a 534-byte descriptor with bLength = 22.

uvc_copy_descriptors() reserves xu->desc.bLength bytes per Extension Unit
in the descriptor buffer it allocates at bind time, but
UVC_COPY_XU_DESCRIPTOR() copies the real descriptor contents, i.e.
22 + bNrInPins + 1 + bControlSize + 1 bytes.  With the wrapped length the
copy overruns the allocation by up to 512 bytes.

Attack chain (write access to a UVC gadget's configfs attributes; no
race, no USB traffic, a single bind triggers it):

    echo 255 > .../functions/uvc.0/extensions/ext.0/b_nr_in_pins
    echo 255 > .../functions/uvc.0/extensions/ext.0/b_control_size
      -> uvcg_extension_b_nr_in_pins_store() /
         uvcg_extension_b_control_size_store()
      -> bLength = UVC_DT_EXTENSION_UNIT_SIZE(255, 255) wraps to 22
    bind the gadget to a UDC
      -> uvc_function_bind() -> uvc_copy_descriptors()
      -> kmalloc() sized using the wrapped bLength: 22 bytes for the XU
      -> UVC_COPY_XU_DESCRIPTOR() writes the real 534 bytes into that
         slot (22-byte head, 255 baSourceID, bControlSize, 255
         bmControls, iExtension)
      -> heap out-of-bounds write during bind

Reproduced on 7.2.0+: a stock build (FORTIFY on) can derive the remaining
allocation size at the bmControls memcpy and BUGs in __fortify_panic()
during bind - a deterministic kernel crash; with FORTIFY disabled for the
file, KASAN reports "slab-out-of-bounds Write of size 255" in
uvc_copy_descriptors() against the kmalloc-192 descriptor buffer.

Reject combinations whose descriptor does not fit into bLength at all four
configfs entry points that can grow an Extension Unit (b_nr_in_pins,
b_control_size, ba_source_id, bm_controls), and make uvc_copy_descriptors()
refuse an Extension Unit whose bLength does not match its contents instead
of overflowing the buffer.

Fixes: 0525210c9840 ("usb: gadget: uvc: Allow definition of XUs in configfs")
Signed-off-by: Haofeng Li <lihaofeng@kylinos.cn>
Assisted-by: opencode:deepseek-v4-flash-free
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
---
Changes in v2:
- reword a comment per Randy Dunlap: "The descriptor length must fit
  into the one-byte bLength field".

Changes in v3:
- Add a warning message when the descriptor size is rejected, as suggested
  by Dan Scally.

Changes in v4:
- Remove the blank line inside the sign-off block.
 drivers/usb/gadget/function/f_uvc.c        |  6 +++++
 drivers/usb/gadget/function/uvc_configfs.c | 28 ++++++++++++++++++++++
 2 files changed, 34 insertions(+)

diff --git a/drivers/usb/gadget/function/f_uvc.c b/drivers/usb/gadget/function/f_uvc.c
index 73dc7e42875f..0d3432f80cae 100644
--- a/drivers/usb/gadget/function/f_uvc.c
+++ b/drivers/usb/gadget/function/f_uvc.c
@@ -598,6 +598,12 @@ uvc_copy_descriptors(struct uvc_device *uvc, enum usb_device_speed speed)
 	}
 
 	list_for_each_entry(xu, uvc->desc.extension_units, list) {
+		/* Mismatched bLength would overflow the buffer sized after it */
+		if (xu->desc.bLength !=
+		    UVC_DT_EXTENSION_UNIT_SIZE(xu->desc.bNrInPins,
+					       xu->desc.bControlSize))
+			return ERR_PTR(-EINVAL);
+
 		control_size += xu->desc.bLength;
 		bytes += xu->desc.bLength;
 		n_desc++;
diff --git a/drivers/usb/gadget/function/uvc_configfs.c b/drivers/usb/gadget/function/uvc_configfs.c
index 70a1415ea401..b52c9cf2b715 100644
--- a/drivers/usb/gadget/function/uvc_configfs.c
+++ b/drivers/usb/gadget/function/uvc_configfs.c
@@ -845,6 +845,18 @@ static ssize_t uvcg_extension_b_num_controls_store(struct config_item *item,
 }
 UVCG_EXTENSION_ATTR(b_num_controls, bNumControls);
 
+/* The descriptor length must fit into the one-byte bLength field */
+static int uvcg_extension_check_size(u8 nr_in_pins, u8 control_size)
+{
+	if (UVC_DT_EXTENSION_UNIT_SIZE(nr_in_pins, control_size) > 255) {
+		pr_warn("uvc: Extension Unit descriptor size (%u + %u) does not fit into bLength\n",
+			nr_in_pins, control_size);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 /*
  * In addition to storing bNrInPins, this function needs to realloc the
  * memory for the baSourceID array and additionally expand bLength.
@@ -877,6 +889,10 @@ static ssize_t uvcg_extension_b_nr_in_pins_store(struct config_item *item,
 		goto unlock;
 	}
 
+	ret = uvcg_extension_check_size(num, xu->desc.bControlSize);
+	if (ret)
+		goto unlock;
+
 	tmp_buf = krealloc_array(xu->desc.baSourceID, num, sizeof(u8),
 				 GFP_KERNEL | __GFP_ZERO);
 	if (!tmp_buf) {
@@ -930,6 +946,10 @@ static ssize_t uvcg_extension_b_control_size_store(struct config_item *item,
 		goto unlock;
 	}
 
+	ret = uvcg_extension_check_size(xu->desc.bNrInPins, num);
+	if (ret)
+		goto unlock;
+
 	tmp_buf = krealloc_array(xu->desc.bmControls, num, sizeof(u8),
 				 GFP_KERNEL | __GFP_ZERO);
 	if (!tmp_buf) {
@@ -1055,6 +1075,10 @@ static ssize_t uvcg_extension_ba_source_id_store(struct config_item *item,
 	if (ret)
 		goto unlock;
 
+	ret = uvcg_extension_check_size(n, xu->desc.bControlSize);
+	if (ret)
+		goto unlock;
+
 	iter = source_ids = kcalloc(n, sizeof(u8), GFP_KERNEL);
 	if (!source_ids) {
 		ret = -ENOMEM;
@@ -1134,6 +1158,10 @@ static ssize_t uvcg_extension_bm_controls_store(struct config_item *item,
 	if (ret)
 		goto unlock;
 
+	ret = uvcg_extension_check_size(xu->desc.bNrInPins, n);
+	if (ret)
+		goto unlock;
+
 	iter = bm_controls = kcalloc(n, sizeof(u8), GFP_KERNEL);
 	if (!bm_controls) {
 		ret = -ENOMEM;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-08-24 17:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-22 19:47 [PATCH] usb: gadget: f_uvc: fix Extension Unit descriptor heap overflow Haofeng Li
2026-08-22 20:01 ` Randy Dunlap
2026-08-22 20:26   ` Haofeng Li
2026-08-22 20:26 ` [PATCH v2] " Haofeng Li
2026-08-24  8:38 ` [PATCH] " Dan Scally
2026-08-24  9:44   ` [PATCH v3] " Haofeng Li
2026-08-24 15:29     ` Andy Shevchenko
2026-08-24 17:04     ` [PATCH v4] " Haofeng Li

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox