[PATCH] usb: gadget: fsl_qe_udc: bound RX frame copies to the request's remaining space

[PATCH] usb: gadget: fsl_qe_udc: bound RX frame copies to the request's remaining space

[Wang Yan]

qe_ep_rxframe_handle() and ep_req_rx() copy the received USB frame into
the active gadget request with

    cp = req->req.buf + req->req.actual;
    if (cp) {
        memcpy(cp, pframe->data, fsize);
        req->req.actual += fsize;
        if (fsize < ep->ep.maxpacket || req->req.actual >= req->req.length)
            /* complete */
    }

where fsize = frame_get_length(pframe) = hardware-reported frame length
minus USB_CRC_SIZE (2).  The length comes from the received OUT packet
on the wire (host-controlled for a device in peripheral mode), while the
request buffer is sized by the gadget driver.  Nothing guarantees
fsize <= req->req.length - req->req.actual, so a frame bigger than the
request's remaining space is memcpy()'d past the buffer, and
req->req.actual is bumped past req->req.length.

Attack chain (device in USB gadget/peripheral mode; attacker is the USB
host):

    malicious host -> OUT packet on a QE/CPM endpoint
      -> qe_udc_irq() -> rx_irq() -> qe_ep0_rx()/qe_ep_rx()
      -> ep_rx_tasklet() -> qe_ep_rxframe_handle() (or ep_req_rx())
      -> fsize = packet length - 2 (e.g. 64 for a 66-byte frame)
      -> request has req.length = 64, req.actual = 63 (1 byte left)
      -> memcpy(req.buf + 63, pframe->data, 64)
      -> 63 bytes past the request buffer

The same correction applies to the ep_req_receive() -> ep_req_rx()
path.  The upstream requests are queued by the gadget layer (ep_queue);
nothing in __qe_ep_queue() guarantees the buffer can absorb one max
packet, so the short-request state is reachable with ordinary gadgets.

Reproduced deterministically (user-space ASAN re-extraction of the
exact source path; the fsl_qe_udc driver cannot be built/loaded on
x86_64 because USB_FSL_QE requires FSL_SOC/QUICC_ENGINE/CPM and
!64BIT): with req.buf = 64 bytes, req.actual = 63 and fsize = 64,
both extracted paths (oepparsing the kernel lines for
qe_ep_rxframe_handle() :914 and ep_req_rx() :1521) fail under
AddressSanitizer with

    ERROR: AddressSanitizer: heap-buffer-overflow ... WRITE of size 64
    located 0 bytes after 64-byte region

while the same code with the remaining-capacity check added produces no
ASAN report at all.

Signed-off-by: Wang Yan <wangyan01@kylinos.cn>
Assisted-by: opencode:deepseek-v4-flash-free
---
 drivers/usb/gadget/udc/fsl_qe_udc.c | 64 ++++++++++++++++++++++++-----
 1 file changed, 53 insertions(+), 11 deletions(-)

diff --git a/drivers/usb/gadget/udc/fsl_qe_udc.c b/drivers/usb/gadget/udc/fsl_qe_udc.c
index 603c77ff129f..bed7edd293a5 100644
--- a/drivers/usb/gadget/udc/fsl_qe_udc.c
+++ b/drivers/usb/gadget/udc/fsl_qe_udc.c
@@ -911,16 +911,37 @@ static int qe_ep_rxframe_handle(struct qe_ep *ep)
 
 		cp = (u8 *)(req->req.buf) + req->req.actual;
 		if (cp) {
-			memcpy(cp, pframe->data, fsize);
-			req->req.actual += fsize;
-			if ((fsize < ep->ep.maxpacket) ||
-					(req->req.actual >= req->req.length)) {
+			if (req->req.actual >= req->req.length ||
+			    fsize > req->req.length - req->req.actual) {
+				/*
+				 * The host sent a frame larger than the
+				 * request can hold; drop it and complete with
+				 * -EOVERFLOW instead of copying past the
+				 * request buffer.
+				 */
+				dev_err(ep->udc->dev,
+					"%s: rx frame %u exceeds remaining %u\n",
+					ep->name, fsize,
+					req->req.length - req->req.actual);
+				qe_frame_clean(pframe);
 				if (ep->epnum == 0)
 					ep0_req_complete(ep->udc, req);
 				else
-					done(ep, req, 0);
+					done(ep, req, -EOVERFLOW);
 				if (list_empty(&ep->queue) && ep->epnum != 0)
 					qe_eprx_nack(ep);
+			} else {
+				memcpy(cp, pframe->data, fsize);
+				req->req.actual += fsize;
+				if ((fsize < ep->ep.maxpacket) ||
+						(req->req.actual >= req->req.length)) {
+					if (ep->epnum == 0)
+						ep0_req_complete(ep->udc, req);
+					else
+						done(ep, req, 0);
+					if (list_empty(&ep->queue) && ep->epnum != 0)
+						qe_eprx_nack(ep);
+				}
 			}
 		}
 	}
@@ -1518,15 +1539,36 @@ static int ep_req_rx(struct qe_ep *ep, struct qe_req *req)
 
 				cp = (u8 *)(req->req.buf) + req->req.actual;
 				if (cp) {
-					memcpy(cp, pframe->data, fsize);
-					req->req.actual += fsize;
-					if ((fsize < ep->ep.maxpacket)
-						|| (req->req.actual >=
-							req->req.length)) {
+					if (req->req.actual >= req->req.length ||
+					    fsize > req->req.length - req->req.actual) {
+						/*
+						 * The host sent a frame
+						 * larger than the request
+						 * can hold; drop it and
+						 * complete with -EOVERFLOW
+						 * instead of copying past
+						 * the request buffer.
+						 */
+						dev_err(udc->dev,
+							"%s: rx frame %u exceeds remaining %u\n",
+							ep->name, fsize,
+							req->req.length - req->req.actual);
+						qe_frame_clean(pframe);
 						finish_req = 1;
-						done(ep, req, 0);
+						done(ep, req, -EOVERFLOW);
 						if (list_empty(&ep->queue))
 							qe_eprx_nack(ep);
+					} else {
+						memcpy(cp, pframe->data, fsize);
+						req->req.actual += fsize;
+						if ((fsize < ep->ep.maxpacket)
+							|| (req->req.actual >=
+								req->req.length)) {
+							finish_req = 1;
+							done(ep, req, 0);
+							if (list_empty(&ep->queue))
+								qe_eprx_nack(ep);
+						}
 					}
 				}
 				qe_ep_toggledata01(ep);
-- 
2.25.1