Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Daehwan Jung <dh10.jung@samsung.com>
To:     Felipe Balbi <balbi@kernel.org>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc:     linux-usb@vger.kernel.org,
        linux-kernel@vger.kernel.org (open list),
        Daehwan Jung <dh10.jung@samsung.com>, quic_wcheng@quicinc.com,
        quic_jackp@quicinc.com, Thinh.Nguyen@synopsys.com
Subject: [PATCH v2 2/2] usb: dwc3: Prevent cleanup cancelled requests at the
 same time.
Date:   Mon, 14 Feb 2022 20:08:53 +0900
Message-Id: <1644836933-141376-3-git-send-email-dh10.jung@samsung.com>
In-Reply-To: <1644836933-141376-1-git-send-email-dh10.jung@samsung.com>
Content-Type: text/plain; charset="utf-8"
CMS-TYPE: 102P
DLP-Filter: Pass
References: <1644836933-141376-1-git-send-email-dh10.jung@samsung.com>
        <CGME20220214111149epcas2p29b9e39b84d7203572422531beb3c39ed@epcas2p2.samsung.com>
Precedence: bulk

We added cleanup cancelled requests when ep cmd timeout on ep dequeue
because there's no complete interrupt then. But, we find out new case
that complete interrupt comes up later. list_for_each_entry_safe is
used when cleanup cancelled requests and it has vulnerabilty on multi-core
environment. dwc3_gadget_giveback unlocks dwc->lock temporarily and other
core(ISR) can get lock and try to cleanup them again. It could cause
list_del corruption and we use DWC3_EP_END_TRANSFER_PENDING to prevent it.

1. MTP server cancels -> ep dequeue -> ep cmd timeout(END_TRANSFER)
	-> cleanup cancelled requests -> dwc3_gadget_giveback ->
	list_del -> release lock temporarily
2. Complete with END_TRANSFER -> ISR(dwc3_gadget_endpoint_command_complete)
	gets lock -> cleanup cancelled requests -> dwc3_gadget_giveback
	-> list_del
3. MTP server process gets lock again
	-> tries to access POISON list(list_del corruption)

[2: MtpServer: 5032] dwc3 10b00000.dwc3: request cancelled
						with wrong reason:5
[2: MtpServer: 5032] list_del corruption,
		ffffff88b6963968->next is LIST_POISON1 (dead000000000100)

Signed-off-by: Daehwan Jung <dh10.jung@samsung.com>
---
 drivers/usb/dwc3/gadget.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index 3ad3bc5813ca..bac3814894e2 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2046,8 +2046,11 @@ static int dwc3_gadget_ep_dequeue(struct usb_ep *ep,
 
 			/* If ep cmd fails, then force to giveback cancelled requests here */
 			if (!(dep->flags & DWC3_EP_END_TRANSFER_PENDING)) {
-				dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
+				dep->flags |= DWC3_EP_END_TRANSFER_PENDING;
 				dwc3_gadget_ep_cleanup_cancelled_requests(dep);
+
+				dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
+				dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING;
 			}
 
 			dep->flags &= ~DWC3_EP_WAIT_TRANSFER_COMPLETE;
@@ -3426,9 +3429,11 @@ static void dwc3_gadget_endpoint_command_complete(struct dwc3_ep *dep,
 	if (dep->stream_capable)
 		dep->flags |= DWC3_EP_IGNORE_NEXT_NOSTREAM;
 
+	if (!(dep->flags & DWC3_EP_END_TRANSFER_PENDING))
+		dwc3_gadget_ep_cleanup_cancelled_requests(dep);
+
 	dep->flags &= ~DWC3_EP_END_TRANSFER_PENDING;
 	dep->flags &= ~DWC3_EP_TRANSFER_STARTED;
-	dwc3_gadget_ep_cleanup_cancelled_requests(dep);
 
 	if (dep->flags & DWC3_EP_PENDING_CLEAR_STALL) {
 		struct dwc3 *dwc = dep->dwc;
-- 
2.31.1