Linux USB
 help / color / mirror / Atom feed
* [PATCH] usb: gadget: f_printer: prevent OOB write in GET_DEVICE_ID
@ 2026-08-21  8:34 Haofeng Li
  2026-08-21 11:01 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 2+ messages in thread
From: Haofeng Li @ 2026-08-21  8:34 UTC (permalink / raw)
  To: Greg Kroah-Hartman
  Cc: Kees Cook, Melbin K Mathew, Christophe JAILLET, Xu Rao, linux-usb,
	linux-kernel, Haofeng Li, Haofeng Li

printer_func_setup() services the Printer Class GET_DEVICE_ID request
by echoing the PnP string previously stored in the gadget's configfs
pnp_string attribute:

	value = strlen(*dev->pnp_string);
	buf[0] = (value >> 8) & 0xFF;
	buf[1] = value & 0xFF;
	memcpy(buf + 2, *dev->pnp_string, value);

The EP0 response buffer is exactly USB_COMP_EP0_BUFSIZ (4096) bytes,
allocated once by composite_dev_prepare():

	cdev->req->buf = kzalloc(USB_COMP_EP0_BUFSIZ, GFP_KERNEL);

The two-byte length prefix plus the string body must therefore fit
into 4096 bytes.  pnp_string is stored via kstrndup(page, len,
GFP_KERNEL) in f_printer_opts_pnp_string_store(); configfs passes at
most PAGE_SIZE - 1 (4095) bytes down to the store callback, so a
4095-byte string combined with the 2-byte length field makes the
memcpy() write buf[2..4096], one byte past the end of the allocation.

Attack chain (USB Printer gadget on the victim device):

  1. pnp_string is set to a 4095-byte value through the gadget's
     configfs attribute
     (~/config/usb_gadget/<gadget>/functions/printer.usb0/pnp_string);
     configfs accepts up to PAGE_SIZE - 1 bytes (fs/configfs/file.c).
  2. The printer function is enabled and the gadget is bound to its
     UDC.  An attacker in control of the connecting USB host sends a
     Printer Class GET_DEVICE_ID request (bmRequestType=0xA1,
     bRequest=0x00, wIndex pointing at the printer interface); the
     usblp host driver also issues this request on enumeration.
  3. composite_setup() -> printer_func_setup() -> memcpy(buf + 2,
     pnp_string, 4095) performs a 4097-byte write into the 4096-byte
     EP0 response buffer, overflowing the heap object by one byte and
     potentially corrupting adjacent slab objects or allocator
     metadata (CWE-787).

With KASAN enabled the overflow is reliably reported (this is
reproducible end to end with a configfs gadget + dummy_hcd):

	BUG: KASAN: slab-out-of-bounds in printer_func_setup+0x2ec/0x3c0
	Write of size 4095 at addr ffff88818e461002

Fix it at both ends:

  - clamp the string length to USB_COMP_EP0_BUFSIZ - 2 in
    printer_func_setup() so the copy can never exceed the EP0 buffer,
    and
  - reject pnp_string values longer than USB_COMP_EP0_BUFSIZ - 2 in
    f_printer_opts_pnp_string_store() so an oversized string is never
    stored in the first place.

Signed-off-by: Haofeng Li <lihaofeng@kylinos.cn>
---
 drivers/usb/gadget/function/f_printer.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/drivers/usb/gadget/function/f_printer.c b/drivers/usb/gadget/function/f_printer.c
index 1857d786110b..4b28e73d35ca 100644
--- a/drivers/usb/gadget/function/f_printer.c
+++ b/drivers/usb/gadget/function/f_printer.c
@@ -1035,6 +1035,17 @@ static int printer_func_setup(struct usb_function *f,
 				break;
 			}
 			value = strlen(*dev->pnp_string);
+			/*
+			 * The EP0 response buffer is USB_COMP_EP0_BUFSIZ
+			 * bytes and the first two bytes hold the string
+			 * length, so at most USB_COMP_EP0_BUFSIZ - 2 bytes
+			 * of the PnP string can be copied.  A string stored
+			 * through configfs is at most USB_COMP_EP0_BUFSIZ - 1
+			 * bytes long, which would overflow the buffer by one
+			 * byte here, so clamp it before the memcpy() below.
+			 */
+			if (value > USB_COMP_EP0_BUFSIZ - 2)
+				value = USB_COMP_EP0_BUFSIZ - 2;
 			buf[0] = (value >> 8) & 0xFF;
 			buf[1] = value & 0xFF;
 			memcpy(buf + 2, *dev->pnp_string, value);
@@ -1269,6 +1280,18 @@ static ssize_t f_printer_opts_pnp_string_store(struct config_item *item,
 
 	mutex_lock(&opts->lock);
 
+	/*
+	 * The string is echoed on the wire by the GET_DEVICE_ID request as
+	 * a two-byte length prefix followed by the string itself, and the
+	 * EP0 response buffer is only USB_COMP_EP0_BUFSIZ bytes, so a
+	 * longer string would make printer_func_setup() overrun that
+	 * buffer.  Reject it here as an additional line of defense.
+	 */
+	if (len > USB_COMP_EP0_BUFSIZ - 2) {
+		result = -EINVAL;
+		goto unlock;
+	}
+
 	new_pnp = kstrndup(page, len, GFP_KERNEL);
 	if (!new_pnp) {
 		result = -ENOMEM;
-- 
2.25.1


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

* Re: [PATCH] usb: gadget: f_printer: prevent OOB write in GET_DEVICE_ID
  2026-08-21  8:34 [PATCH] usb: gadget: f_printer: prevent OOB write in GET_DEVICE_ID Haofeng Li
@ 2026-08-21 11:01 ` Greg Kroah-Hartman
  0 siblings, 0 replies; 2+ messages in thread
From: Greg Kroah-Hartman @ 2026-08-21 11:01 UTC (permalink / raw)
  To: Haofeng Li
  Cc: Kees Cook, Melbin K Mathew, Christophe JAILLET, Xu Rao, linux-usb,
	linux-kernel, Haofeng Li

On Fri, Aug 21, 2026 at 04:34:28PM +0800, Haofeng Li wrote:
> printer_func_setup() services the Printer Class GET_DEVICE_ID request
> by echoing the PnP string previously stored in the gadget's configfs
> pnp_string attribute:
> 
> 	value = strlen(*dev->pnp_string);
> 	buf[0] = (value >> 8) & 0xFF;
> 	buf[1] = value & 0xFF;
> 	memcpy(buf + 2, *dev->pnp_string, value);
> 
> The EP0 response buffer is exactly USB_COMP_EP0_BUFSIZ (4096) bytes,
> allocated once by composite_dev_prepare():
> 
> 	cdev->req->buf = kzalloc(USB_COMP_EP0_BUFSIZ, GFP_KERNEL);
> 
> The two-byte length prefix plus the string body must therefore fit
> into 4096 bytes.  pnp_string is stored via kstrndup(page, len,
> GFP_KERNEL) in f_printer_opts_pnp_string_store(); configfs passes at
> most PAGE_SIZE - 1 (4095) bytes down to the store callback, so a
> 4095-byte string combined with the 2-byte length field makes the
> memcpy() write buf[2..4096], one byte past the end of the allocation.
> 
> Attack chain (USB Printer gadget on the victim device):
> 
>   1. pnp_string is set to a 4095-byte value through the gadget's
>      configfs attribute
>      (~/config/usb_gadget/<gadget>/functions/printer.usb0/pnp_string);
>      configfs accepts up to PAGE_SIZE - 1 bytes (fs/configfs/file.c).
>   2. The printer function is enabled and the gadget is bound to its
>      UDC.  An attacker in control of the connecting USB host sends a
>      Printer Class GET_DEVICE_ID request (bmRequestType=0xA1,
>      bRequest=0x00, wIndex pointing at the printer interface); the
>      usblp host driver also issues this request on enumeration.
>   3. composite_setup() -> printer_func_setup() -> memcpy(buf + 2,
>      pnp_string, 4095) performs a 4097-byte write into the 4096-byte
>      EP0 response buffer, overflowing the heap object by one byte and
>      potentially corrupting adjacent slab objects or allocator
>      metadata (CWE-787).
> 
> With KASAN enabled the overflow is reliably reported (this is
> reproducible end to end with a configfs gadget + dummy_hcd):
> 
> 	BUG: KASAN: slab-out-of-bounds in printer_func_setup+0x2ec/0x3c0
> 	Write of size 4095 at addr ffff88818e461002
> 
> Fix it at both ends:
> 
>   - clamp the string length to USB_COMP_EP0_BUFSIZ - 2 in
>     printer_func_setup() so the copy can never exceed the EP0 buffer,
>     and
>   - reject pnp_string values longer than USB_COMP_EP0_BUFSIZ - 2 in
>     f_printer_opts_pnp_string_store() so an oversized string is never
>     stored in the first place.
> 
> Signed-off-by: Haofeng Li <lihaofeng@kylinos.cn>
> ---
>  drivers/usb/gadget/function/f_printer.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)

Did you forget an Assisted-by: tag here?

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

end of thread, other threads:[~2026-08-21 11:02 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-21  8:34 [PATCH] usb: gadget: f_printer: prevent OOB write in GET_DEVICE_ID Haofeng Li
2026-08-21 11:01 ` Greg Kroah-Hartman

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