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>] [day] [month] [year] [list]
Date:   Thu, 14 Apr 2022 00:39:02 -0700
From:   Wesley Cheng <quic_wcheng@...cinc.com>
To:     <balbi@...nel.org>, <gregkh@...uxfoundation.org>
CC:     <linux-kernel@...r.kernel.org>, <linux-usb@...r.kernel.org>,
        <quic_jackp@...cinc.com>, <quic_pkondeti@...cinc.com>,
        <Thinh.Nguyen@...opsys.com>, Wesley Cheng <quic_wcheng@...cinc.com>
Subject: [PATCH v2] usb: dwc3: EP clear halt leading to clearing of delayed_status

The usb_ep_clear_halt() API can be called from the function driver, and
translates to dwc3_gadget_ep_set_halt().  This routine is shared with when
the host issues a clear feature ENDPOINT_HALT, and is differentiated by the
protocol argument.  If the following sequence occurs, there can be a
situation where the delayed_status flag is improperly cleared for the wrong
SETUP transaction:

1. Vendor specific control transfer returns USB_GADGET_DELAYED_STATUS.
2. DWC3 gadget sets dwc->delayed_status to '1'.
3. Another function driver issues a usb_ep_clear_halt() call.
4. DWC3 gadget issues dwc3_stop_active_transfer() and sets
   DWC3_EP_PENDING_CLEAR_STALL.
5. EP command complete interrupt triggers for the end transfer, and
   dwc3_ep0_send_delayed_status() is allowed to run, as delayed_status
   is '1' due to step#1.
6. STATUS phase is sent, and delayed_status is cleared.
7. Vendor specific control transfer is finished being handled, and issues
   usb_composite_setup_continue().  This results in queuing of a data
   phase.

Cache the protocol flag so that DWC3 gadget is aware of when the clear halt
is due to a SETUP request from the host versus when it is sourced from a
function driver.  This allows for the EP command complete interrupt to know
if it needs to issue a delayed status phase.

Signed-off-by: Wesley Cheng <quic_wcheng@...cinc.com>
---
Changes in v2:
 - Modified clear_stall_protocol to carry the ep num for the clear halt
   from PC.  Clear halts on other endpoints in parallel should not
   affect/interrupt the clear halt from the host.

 drivers/usb/dwc3/core.h   | 2 ++
 drivers/usb/dwc3/ep0.c    | 1 +
 drivers/usb/dwc3/gadget.c | 5 ++++-
 3 files changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/usb/dwc3/core.h b/drivers/usb/dwc3/core.h
index 5c9d467195a6..81c486b3941c 100644
--- a/drivers/usb/dwc3/core.h
+++ b/drivers/usb/dwc3/core.h
@@ -1046,6 +1046,7 @@ struct dwc3_scratchpad_array {
  * @tx_thr_num_pkt_prd: periodic ESS transmit packet count
  * @tx_max_burst_prd: max periodic ESS transmit burst size
  * @tx_fifo_resize_max_num: max number of fifos allocated during txfifo resize
+ * @clear_stall_protocol: endpoint number that requires a delayed status phase
  * @hsphy_interface: "utmi" or "ulpi"
  * @connected: true when we're connected to a host, false otherwise
  * @softconnect: true when gadget connect is called, false when disconnect runs
@@ -1266,6 +1267,7 @@ struct dwc3 {
 	u8			tx_thr_num_pkt_prd;
 	u8			tx_max_burst_prd;
 	u8			tx_fifo_resize_max_num;
+	u8			clear_stall_protocol;
 
 	const char		*hsphy_interface;
 
diff --git a/drivers/usb/dwc3/ep0.c b/drivers/usb/dwc3/ep0.c
index 1064be5518f6..aa8476da222d 100644
--- a/drivers/usb/dwc3/ep0.c
+++ b/drivers/usb/dwc3/ep0.c
@@ -1080,6 +1080,7 @@ void dwc3_ep0_send_delayed_status(struct dwc3 *dwc)
 	unsigned int direction = !dwc->ep0_expect_in;
 
 	dwc->delayed_status = false;
+	dwc->clear_stall_protocol = 0;
 
 	if (dwc->ep0state != EP0_STATUS_PHASE)
 		return;
diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
index ab725d2262d6..ce1b130202c5 100644
--- a/drivers/usb/dwc3/gadget.c
+++ b/drivers/usb/dwc3/gadget.c
@@ -2152,6 +2152,9 @@ int __dwc3_gadget_ep_set_halt(struct dwc3_ep *dep, int value, int protocol)
 		if (dep->flags & DWC3_EP_END_TRANSFER_PENDING ||
 		    (dep->flags & DWC3_EP_DELAY_STOP)) {
 			dep->flags |= DWC3_EP_PENDING_CLEAR_STALL;
+			if (protocol)
+				dwc->clear_stall_protocol = dep->number;
+
 			return 0;
 		}
 
@@ -3483,7 +3486,7 @@ static void dwc3_gadget_endpoint_command_complete(struct dwc3_ep *dep,
 		}
 
 		dep->flags &= ~(DWC3_EP_STALL | DWC3_EP_WEDGE);
-		if (dwc->delayed_status)
+		if (dwc->clear_stall_protocol == dep->number)
 			dwc3_ep0_send_delayed_status(dwc);
 	}
 

Powered by blists - more mailing lists