[PATCH v1] usb: cdns3: Fix WA2 request UAF on endpoint disable

[PATCH v1] usb: cdns3: Fix WA2 request UAF on endpoint disable

[Jaeyeong Lee]

WA2 internal requests use two independent list nodes. priv_req->list
links a request into wa2_descmiss_req_list, while request.list links the
same request into the pending or deferred request list.

If an endpoint is disabled while an internal request is still deferred,
cdns3_gadget_ep_disable() removes only priv_req->list before freeing the
request. The following deferred-list cleanup then passes the freed
request to cdns3_gadget_giveback(), where list_del_init() accesses freed
memory.

Initialize request.list when adding an internal request to the WA2 list,
and remove it before freeing the request during endpoint disable. This
also keeps the cleanup valid when queue setup fails before request.list
is added to a request queue.

A QEMU KASAN reproducer placed an internal request on both lists and
invoked cdns3_gadget_ep_disable(). The unpatched kernel reported:

  BUG: KASAN: slab-use-after-free in __list_del_entry_valid_or_report
  cdns3_gadget_giveback
  cdns3_gadget_ep_disable

The fixed kernel completed endpoint cleanup without a KASAN or
DEBUG_LIST report.

The issue was found with a static analysis tool.

Fixes: 6bbf87a131ed ("usb:cdns3 Fix for stuck packets in on-chip OUT buffer.")
Cc: stable@vger.kernel.org
Signed-off-by: Jaeyeong Lee <lee@jaeyeong.cc>
---
 drivers/usb/cdns3/cdns3-gadget.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c
index f4dfd01317083..70968e8f0d2da 100644
--- a/drivers/usb/cdns3/cdns3-gadget.c
+++ b/drivers/usb/cdns3/cdns3-gadget.c
@@ -605,9 +605,11 @@ static int cdns3_wa2_gadget_ep_queue(struct cdns3_device *priv_dev,
 			deferred = 1;
 		}
 
-		if (priv_req->flags & REQUEST_INTERNAL)
+		if (priv_req->flags & REQUEST_INTERNAL) {
+			INIT_LIST_HEAD(&priv_req->request.list);
 			list_add_tail(&priv_req->list,
 				      &priv_ep->wa2_descmiss_req_list);
+		}
 	}
 
 	return deferred;
@@ -2547,6 +2549,7 @@ static int cdns3_gadget_ep_disable(struct usb_ep *ep)
 	while (!list_empty(&priv_ep->wa2_descmiss_req_list)) {
 		priv_req = cdns3_next_priv_request(&priv_ep->wa2_descmiss_req_list);
 		list_del_init(&priv_req->list);
+		list_del_init(&priv_req->request.list);
 
 		kfree(priv_req->request.buf);
 		cdns3_gadget_ep_free_request(&priv_ep->endpoint,
-- 
2.43.0

Re: [PATCH v1] usb: cdns3: Fix WA2 request UAF on endpoint disable

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] usb: cdns3: missing DMA unmap for deferred WA2 requests in
  cdns3_gadget_ep_disable()
--

commit 15008f28246eb4c2cca9af667c3695fb8684956a
Author: Jaeyeong Lee <lee@jaeyeong.cc>

usb: cdns3: Fix WA2 request UAF on endpoint disable

This commit initializes priv_req->request.list when adding an internal WA2
request to wa2_descmiss_req_list and removes it during
cdns3_gadget_ep_disable() before freeing the request.

> diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3-gadget.c
> index 380c4fde8298e..770fbeee53484 100644
> --- a/drivers/usb/cdns3/cdns3-gadget.c
> +++ b/drivers/usb/cdns3/cdns3-gadget.c

[ ... ]

> @@ -2547,6 +2549,7 @@ static int cdns3_gadget_ep_disable(struct usb_ep *ep)
>  	while (!list_empty(&priv_ep->wa2_descmiss_req_list)) {
>  		priv_req = cdns3_next_priv_request(&priv_ep->wa2_descmiss_req_list);
>  		list_del_init(&priv_req->list);
> +		list_del_init(&priv_req->request.list);
>  
>  		kfree(priv_req->request.buf);
>  		cdns3_gadget_ep_free_request(&priv_ep->endpoint,

[Severity: Medium]
Does this free priv_req->request.buf while it is still mapped for DMA when
an internal WA2 request is on priv_ep->deferred_req_list?

__cdns3_gadget_ep_queue() calls usb_gadget_map_request_by_dev() before
adding priv_req->request.list to priv_ep->deferred_req_list.  Requests on
priv_ep->pending_req_list are unmapped by cdns3_gadget_giveback() in the
preceding loop, but unlinking priv_req->request.list here removes a
deferred WA2 request before the deferred_req_list giveback loop runs,
leaking the DMA mapping and triggering a DMA-API warning when
priv_req->request.buf is freed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111132346.9262-1-lee@jaeyeong.cc?part=1