linux-kernel - [RFC][PATCH] dwc3: gadget: Fix losing list items in dwc3_gadget_ep_cleanup_completed

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20210809223159.2342385-1-john.stultz@linaro.org>
Date:   Mon,  9 Aug 2021 22:31:59 +0000
From:   John Stultz <john.stultz@...aro.org>
To:     lkml <linux-kernel@...r.kernel.org>
Cc:     John Stultz <john.stultz@...aro.org>,
        Wesley Cheng <wcheng@...eaurora.org>,
        Felipe Balbi <balbi@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Alan Stern <stern@...land.harvard.edu>,
        Jack Pham <jackp@...eaurora.org>,
        Thinh Nguyen <thinh.nguyen@...opsys.com>,
        Todd Kjos <tkjos@...gle.com>,
        Amit Pundir <amit.pundir@...aro.org>,
        YongQin Liu <yongqin.liu@...aro.org>,
        Sumit Semwal <sumit.semwal@...aro.org>,
        Petri Gynther <pgynther@...gle.com>, linux-usb@...r.kernel.org
Subject: [RFC][PATCH] dwc3: gadget: Fix losing list items in dwc3_gadget_ep_cleanup_completed_requests()

In commit d25d85061bd8 ("usb: dwc3: gadget: Use
list_replace_init() before traversing lists"), a local list_head
was introduced to process the started_list items to avoid races.

However, in dwc3_gadget_ep_cleanup_completed_requests() if
dwc3_gadget_ep_cleanup_completed_request() fails, we break early,
causing the items on the local list_head to be lost.

This issue showed up as problems on the db845c/RB3 board, where
adb connetions would fail, showing the device as "offline".

This patch tries to fix the issue by if we are returning early
we splice in the local list head back into the started_list
and return (avoiding an infinite loop, as the started_list is
now non-null).

Not sure if this is fully correct, but seems to work for me so I
wanted to share for feedback.

Cc: Wesley Cheng <wcheng@...eaurora.org>
Cc: Felipe Balbi <balbi@...nel.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Alan Stern <stern@...land.harvard.edu>
Cc: Jack Pham <jackp@...eaurora.org>
Cc: Thinh Nguyen <thinh.nguyen@...opsys.com>
Cc: Todd Kjos <tkjos@...gle.com>
Cc: Amit Pundir <amit.pundir@...aro.org>
Cc: YongQin Liu <yongqin.liu@...aro.org>
Cc: Sumit Semwal <sumit.semwal@...aro.org>
Cc: Petri Gynther <pgynther@...gle.com>
Cc: linux-usb@...r.kernel.org
Fixes: d25d85061bd8 ("usb: dwc3: gadget: Use list_replace_init() before traversing lists")
Signed-off-by: John Stultz <john.stultz@...aro.org>
---
 drivers/usb/dwc3/gadget.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index b8d4b2d327b23..a73ebe8e75024 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2990,6 +2990,12 @@ static void dwc3_gadget_ep_cleanup_completed_requests(struct dwc3_ep *dep,
 			break;
 	}
 
+	if (!list_empty(&local)) {
+		list_splice_tail(&local, &dep->started_list);
+		/* Return so we don't hit the restart case and loop forever */
+		return;
+	}
+
 	if (!list_empty(&dep->started_list))
 		goto restart;
 }
-- 
2.25.1