[PATCH] usb: mtu3: cancel EP0 requests to prevent use-after-free

[PATCH] usb: mtu3: cancel EP0 requests to prevent use-after-free

[Fangwen Deng]

mtu3_ep0_dequeue() always returns -EINVAL and leaves the request on
mep->req_list without unlinking or completing it, so an EP0 request
can never be cancelled.

FunctionFS assigns its temporary read buffer to ep0req and waits in
__ffs_ep0_queue_wait(). When that wait is interrupted by a signal it
calls usb_ep_dequeue(), does not check the return value, and its
caller ffs_ep0_read() frees the buffer on every exit path. With
-EINVAL returned the request stays queued and MTU3 still owns it.
When the host then sends the OUT data stage, ep0_rx_state() writes
the received bytes through the stale req->buf, giving the host a
content-controlled write into freed kernel memory. That corrupts
whichever object reuses the slab and can panic the kernel.

Implement mtu3_ep0_dequeue() following mtu3_gadget_dequeue() but
without QMU flush/start since EP0 uses PIO: validate the request
under mtu->lock, unlink it once and give it back with -ECONNRESET,
so ownership of the buffer returns to FunctionFS before it is freed.

Cancelling the request that backed the data stage also leaves the
transfer unable to finish, so retire the aborted transaction with a
protocol stall. That returns ep0_state to SETUP and stops a later
ep0_queue() from reading this transaction's leftover FIFO data into
the next request's buffer. The stall is skipped when the controller
is already powered down or no data stage is in flight.

The same stub is why functionfs_unbind() cannot cancel a pending EP0
request, which deadlocks against the FunctionFS thread holding
ffs->mutex.

Fixes: df2069acb005 ("usb: Add MediaTek USB3 DRD driver")
Signed-off-by: Fangwen Deng <fangwen.deng@mediatek.com>
---
 drivers/usb/mtu3/mtu3_gadget_ep0.c | 55 ++++++++++++++++++++++++++++--
 1 file changed, 53 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/mtu3/mtu3_gadget_ep0.c b/drivers/usb/mtu3/mtu3_gadget_ep0.c
index e4fd1bb14a55..73a1af8b6c65 100644
--- a/drivers/usb/mtu3/mtu3_gadget_ep0.c
+++ b/drivers/usb/mtu3/mtu3_gadget_ep0.c
@@ -857,8 +857,59 @@ static int mtu3_ep0_queue(struct usb_ep *ep,
 
 static int mtu3_ep0_dequeue(struct usb_ep *ep, struct usb_request *req)
 {
-	/* we just won't support this */
-	return -EINVAL;
+	struct mtu3_ep *mep;
+	struct mtu3_request *mreq = NULL;
+	struct mtu3_request *r = NULL;
+	struct mtu3 *mtu;
+	unsigned long flags;
+	int ret = 0;
+
+	if (!ep || !req)
+		return -EINVAL;
+
+	mep = to_mtu3_ep(ep);
+	mtu = mep->mtu;
+
+	spin_lock_irqsave(&mtu->lock, flags);
+
+	mreq = to_mtu3_request(req);
+
+	dev_dbg(mtu->dev, "%s : req=%p\n", __func__, req);
+
+	list_for_each_entry(r, &mep->req_list, list) {
+		if (r == mreq)
+			break;
+	}
+	if (r != mreq) {
+		dev_info(mtu->dev, "req=%p not queued to %s\n", req, ep->name);
+		ret = -EINVAL;
+		goto done;
+	}
+
+	if (mreq->mep != mep) {
+		dev_info(mtu->dev, "mreq->mep != mep, not queued.\n");
+		ret = -EINVAL;
+		goto done;
+	}
+
+	trace_mtu3_gadget_dequeue(mreq);
+
+	/*
+	 * The cancelled request backed the current control transfer, so abort it
+	 * with an EP0 STALL and reset ep0_state to SETUP to avoid FIFO leftovers
+	 * leaking into the next request. Clear happens in the EP0 ISR (W1C
+	 * EP0_SENTSTALL), so EP0 won't stay halted. Do this before
+	 * mtu3_req_complete() drops mtu->lock to avoid stalling a new transfer.
+	 */
+	if (mtu->is_active && mtu->ep0_state != MU3D_EP0_STATE_SETUP)
+		ep0_stall_set(mtu->ep0, true, 0);
+
+	/* ep0 uses PIO, so no QMU to flush/restart as mtu3_gadget_dequeue() does */
+	mtu3_req_complete(mep, req, -ECONNRESET);
+
+done:
+	spin_unlock_irqrestore(&mtu->lock, flags);
+	return ret;
 }
 
 static int mtu3_ep0_halt(struct usb_ep *ep, int value)
-- 
2.45.2

Re: [PATCH] usb: mtu3: cancel EP0 requests to prevent use-after-free

[sashiko-bot]

> mtu3_ep0_dequeue() always returns -EINVAL and leaves the request on
> mep->req_list without unlinking or completing it, so an EP0 request
> can never be cancelled.
> 
> FunctionFS assigns its temporary read buffer to ep0req and waits in
> __ffs_ep0_queue_wait(). When that wait is interrupted by a signal it
> calls usb_ep_dequeue(), does not check the return value, and its
> caller ffs_ep0_read() frees the buffer on every exit path. With
> -EINVAL returned the request stays queued and MTU3 still owns it.
> When the host then sends the OUT data stage, ep0_rx_state() writes
> the received bytes through the stale req->buf, giving the host a
> content-controlled write into freed kernel memory. That corrupts
> whichever object reuses the slab and can panic the kernel.
> 
> Implement mtu3_ep0_dequeue() following mtu3_gadget_dequeue() but
> [ ... ]
> request, which deadlocks against the FunctionFS thread holding
> ffs->mutex.
> 
> Fixes: df2069acb005 ("usb: Add MediaTek USB3 DRD driver")
> Signed-off-by: Fangwen Deng <fangwen.deng@mediatek.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106080140.342107-1-fangwen.deng@mediatek.com?part=1