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

Wang Yan <[email protected]>
Newsgroups gmane.linux.ports.ppc.embedded
Message-ID <20260828083445.221036-1-wangyan01__27285.2125972108$1787906208$gmane$org@kylinos.cn>
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 <[email protected]>
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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.