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

["Pandey, Radhey Shyam" <radheys@amd.com>]

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: