[PATCH v2] usb: gadget: f_uvc: fix Extension Unit descriptor heap overflow

Haofeng Li <lihaofeng-UOlijcLmZ/[email protected]>
Newsgroups gmane.linux.usb.general,gmane.linux.kernel
Message-ID <[email protected]>
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-UOlijcLmZ/[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.