[PATCH] usb: gadget: udc-xilinx: cap non-EP0 OUT transfers to the request buffer
Pandey, Radhey Shyam
radheys at amd.com
Sun Sep 6 05:26:55 PDT 2026
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 at 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:
More information about the linux-arm-kernel
mailing list