[PATCH] usb: gadget: udc-xilinx: cap non-EP0 OUT transfers to the request buffer

[PATCH] usb: gadget: udc-xilinx: cap non-EP0 OUT transfers to the request buffer

[Haofeng Li]

xudc_read_fifo() takes the transfer size from the endpoint count
register (device/host-controlled for OUT packets) and, if the request
still has some room (bufferspace != 0), passes it straight to
xudc_eptxrx():

    count = udc->read_fn(udc->addr + ep->offset + bufoffset);
    bufferspace = req->usb_req.length - req->usb_req.actual;
    if (!bufferspace) { ... }                /* only the ==0 case handled */
    ret = xudc_eptxrx(ep, req, buf, count);  /* PIO: memcpy_toio(buf, epram, count) */
    req->usb_req.actual += min(count, bufferspace);

There is no cap of count against bufferspace.  The PIO path copies the
full count bytes into req->usb_req.buf + actual (memcpy_toio), and the
DMA path programs a DMA of length count - both will overflow the
request buffer whenever count > bufferspace, with only the bookkeeping
(actual += min(count, bufferspace)) clamped afterwards.

Attack chain (USB peripheral mode, non-EP0 OUT endpoint; attacker is
the USB host):

    malicious host -> OUT packet on epX -> UDC interrupt
      -> xudc_read_fifo() -> count = endpoint count register (e.g. 64)
      -> request with req.length = 8, req.actual = 7 (1 byte left)
      -> bufferspace = 1, count > bufferspace, bufferspace != 0
      -> xudc_eptxrx() PIO OUT: memcpy_toio(req.buf + 7, epram, 64)
      -> 63 bytes past the 8-byte request buffer
      (dwc2-style FunctionFS / f_tcm gadgets submit short requests, so
       the interface does not guarantee a buffer as large as the
       endpoint max packet size.)

Reproduced (kernel 7.2.0+, KASAN/SLUB debug): calling the real
xudc_read_fifo() with the above state leaves req->usb_req.actual = 8
(count 64, bufferspace 1, actual += min = 1) while the PIO copy has
already written 64 bytes: byte 7 of the 8-byte buffer changes from the
0xBB marker to FIFO data and the SLUB redzone plus neighbouring slab
objects past the buffer are overwritten (65 of 72 dumped bytes differ
from the pre-use pattern) - the out-of-bounds write is observed
byte-for-byte.  (memcpy_toio goes through an architecture copy routine
that generic KASAN does not instrument, so the surrounding-memory clobber
is the forensic evidence.)

Signed-off-by: Haofeng Li <lihaofeng@kylinos.cn>
Assisted-by: opencode:deepseek-v4-flash-free
---
 drivers/usb/gadget/udc/udc-xilinx.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/drivers/usb/gadget/udc/udc-xilinx.c b/drivers/usb/gadget/udc/udc-xilinx.c
index bef06fe7543b..63749fd97625 100644
--- a/drivers/usb/gadget/udc/udc-xilinx.c
+++ b/drivers/usb/gadget/udc/udc-xilinx.c
@@ -627,6 +627,24 @@ static int xudc_read_fifo(struct xusb_ep *ep, struct xusb_req *req)
 		return 0;
 	}
 
+	if (count > bufferspace) {
+		/*
+		 * The host sent a packet larger than the request buffer.
+		 * The PIO path would memcpy_toio() the whole packet into
+		 * req->usb_req.buf (and the DMA path would program a DMA
+		 * transfer of count bytes), overflowing the buffer; only the
+		 * bookkeeping actual += min(count, bufferspace) afterwards
+		 * would be clamped.  Complete the request with -EOVERFLOW
+		 * instead of copying past it.
+		 */
+		if (req->usb_req.status != -EOVERFLOW)
+			dev_dbg(udc->dev, "%s overflow %d into %u\n",
+				ep->ep_usb.name, count, bufferspace);
+		req->usb_req.status = -EOVERFLOW;
+		xudc_done(ep, req, -EOVERFLOW);
+		return 0;
+	}
+
 	ret = xudc_eptxrx(ep, req, buf, count);
 	switch (ret) {
 	case 0:
-- 
2.25.1

Re: [PATCH] usb: gadget: udc-xilinx: cap non-EP0 OUT transfers to the request buffer

[Pandey, Radhey Shyam]

On 8/26/2026 7:06 PM, Haofeng Li wrote:
> xudc_read_fifo() takes the transfer size from the endpoint count
> register (device/host-controlled for OUT packets) and, if the request

only host controlled ?

> still has some room (bufferspace != 0), passes it straight to
> xudc_eptxrx():
> 
>      count = udc->read_fn(udc->addr + ep->offset + bufoffset);
>      bufferspace = req->usb_req.length - req->usb_req.actual;
>      if (!bufferspace) { ... }                /* only the ==0 case handled */
>      ret = xudc_eptxrx(ep, req, buf, count);  /* PIO: memcpy_toio(buf, epram, count) */
>      req->usb_req.actual += min(count, bufferspace);
> 
> There is no cap of count against bufferspace.  The PIO path copies the
> full count bytes into req->usb_req.buf + actual (memcpy_toio), and the
> DMA path programs a DMA of length count - both will overflow the
> request buffer whenever count > bufferspace, with only the bookkeeping
> (actual += min(count, bufferspace)) clamped afterwards.
> 
> Attack chain (USB peripheral mode, non-EP0 OUT endpoint; attacker is
> the USB host):
> 
>      malicious host -> OUT packet on epX -> UDC interrupt
>        -> xudc_read_fifo() -> count = endpoint count register (e.g. 64)
>        -> request with req.length = 8, req.actual = 7 (1 byte left)
>        -> bufferspace = 1, count > bufferspace, bufferspace != 0
>        -> xudc_eptxrx() PIO OUT: memcpy_toio(req.buf + 7, epram, 64)
>        -> 63 bytes past the 8-byte request buffer
>        (dwc2-style FunctionFS / f_tcm gadgets submit short requests, so
>         the interface does not guarantee a buffer as large as the
>         endpoint max packet size.)
> 
> Reproduced (kernel 7.2.0+, KASAN/SLUB debug): calling the real
> xudc_read_fifo() with the above state leaves req->usb_req.actual = 8
> (count 64, bufferspace 1, actual += min = 1) while the PIO copy has
> already written 64 bytes: byte 7 of the 8-byte buffer changes from the
> 0xBB marker to FIFO data and the SLUB redzone plus neighbouring slab
> objects past the buffer are overwritten (65 of 72 dumped bytes differ
> from the pre-use pattern) - the out-of-bounds write is observed
> byte-for-byte.  (memcpy_toio goes through an architecture copy routine
> that generic KASAN does not instrument, so the surrounding-memory clobber
> is the forensic evidence.)
> 
> Signed-off-by: Haofeng Li <lihaofeng@kylinos.cn>
> Assisted-by: opencode:deepseek-v4-flash-free
> ---
>   drivers/usb/gadget/udc/udc-xilinx.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 
> diff --git a/drivers/usb/gadget/udc/udc-xilinx.c b/drivers/usb/gadget/udc/udc-xilinx.c
> index bef06fe7543b..63749fd97625 100644
> --- a/drivers/usb/gadget/udc/udc-xilinx.c
> +++ b/drivers/usb/gadget/udc/udc-xilinx.c
> @@ -627,6 +627,24 @@ static int xudc_read_fifo(struct xusb_ep *ep, struct xusb_req *req)
>   		return 0;
>   	}
>   
> +	if (count > bufferspace) {

Can we merge this check with above . It should avoid duplication?

-   if (unlikely(!bufferspace)) {
+   if (unlikely(!bufferspace || count > bufferspace)) {

In v2 , also add fixes and stable kernel tag.

> +		/*
> +		 * The host sent a packet larger than the request buffer.
> +		 * The PIO path would memcpy_toio() the whole packet into
> +		 * req->usb_req.buf (and the DMA path would program a DMA
> +		 * transfer of count bytes), overflowing the buffer; only the
> +		 * bookkeeping actual += min(count, bufferspace) afterwards
> +		 * would be clamped.  Complete the request with -EOVERFLOW
> +		 * instead of copying past it.
> +		 */
> +		if (req->usb_req.status != -EOVERFLOW)
> +			dev_dbg(udc->dev, "%s overflow %d into %u\n",
> +				ep->ep_usb.name, count, bufferspace);
> +		req->usb_req.status = -EOVERFLOW;
> +		xudc_done(ep, req, -EOVERFLOW);
> +		return 0;
> +	}
> +
>   	ret = xudc_eptxrx(ep, req, buf, count);
>   	switch (ret) {
>   	case 0: