From: Sven Peter sven@svenpeter.dev
commit 62c73bfea048e66168df09da6d3e4510ecda40bb upstream.
dwc->desired_dr_role is changed by dwc3_set_mode inside a spinlock but then read by __dwc3_set_mode outside of that lock. This can lead to a race condition when very quick successive role switch events happen:
CPU A dwc3_set_mode(DWC3_GCTL_PRTCAP_HOST) // first role switch event spin_lock_irqsave(&dwc->lock, flags); dwc->desired_dr_role = mode; // DWC3_GCTL_PRTCAP_HOST spin_unlock_irqrestore(&dwc->lock, flags); queue_work(system_freezable_wq, &dwc->drd_work);
CPU B __dwc3_set_mode // .... spin_lock_irqsave(&dwc->lock, flags); // desired_dr_role is DWC3_GCTL_PRTCAP_HOST dwc3_set_prtcap(dwc, dwc->desired_dr_role); spin_unlock_irqrestore(&dwc->lock, flags);
CPU A dwc3_set_mode(DWC3_GCTL_PRTCAP_DEVICE) // second event spin_lock_irqsave(&dwc->lock, flags); dwc->desired_dr_role = mode; // DWC3_GCTL_PRTCAP_DEVICE spin_unlock_irqrestore(&dwc->lock, flags);
CPU B (continues running __dwc3_set_mode) switch (dwc->desired_dr_role) { // DWC3_GCTL_PRTCAP_DEVICE // .... case DWC3_GCTL_PRTCAP_DEVICE: // .... ret = dwc3_gadget_init(dwc);
We then have DWC3_GCTL.DWC3_GCTL_PRTCAPDIR = DWC3_GCTL_PRTCAP_HOST and dwc->current_dr_role = DWC3_GCTL_PRTCAP_HOST but initialized the controller in device mode. It's also possible to get into a state where both host and device are intialized at the same time. Fix this race by creating a local copy of desired_dr_role inside __dwc3_set_mode while holding dwc->lock.
Fixes: 41ce1456e1db ("usb: dwc3: core: make dwc3_set_mode() work properly") Cc: stable stable@kernel.org Acked-by: Thinh Nguyen Thinh.Nguyen@synopsys.com Signed-off-by: Sven Peter sven@svenpeter.dev Link: https://lore.kernel.org/r/20221128161526.79730-1-sven@svenpeter.dev Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org --- drivers/usb/dwc3/core.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-)
--- a/drivers/usb/dwc3/core.c +++ b/drivers/usb/dwc3/core.c @@ -120,21 +120,25 @@ static void __dwc3_set_mode(struct work_ unsigned long flags; int ret; u32 reg; + u32 desired_dr_role;
mutex_lock(&dwc->mutex); + spin_lock_irqsave(&dwc->lock, flags); + desired_dr_role = dwc->desired_dr_role; + spin_unlock_irqrestore(&dwc->lock, flags);
pm_runtime_get_sync(dwc->dev);
if (dwc->current_dr_role == DWC3_GCTL_PRTCAP_OTG) dwc3_otg_update(dwc, 0);
- if (!dwc->desired_dr_role) + if (!desired_dr_role) goto out;
- if (dwc->desired_dr_role == dwc->current_dr_role) + if (desired_dr_role == dwc->current_dr_role) goto out;
- if (dwc->desired_dr_role == DWC3_GCTL_PRTCAP_OTG && dwc->edev) + if (desired_dr_role == DWC3_GCTL_PRTCAP_OTG && dwc->edev) goto out;
switch (dwc->current_dr_role) { @@ -162,7 +166,7 @@ static void __dwc3_set_mode(struct work_ */ if (dwc->current_dr_role && ((DWC3_IP_IS(DWC3) || DWC3_VER_IS_PRIOR(DWC31, 190A)) && - dwc->desired_dr_role != DWC3_GCTL_PRTCAP_OTG)) { + desired_dr_role != DWC3_GCTL_PRTCAP_OTG)) { reg = dwc3_readl(dwc->regs, DWC3_GCTL); reg |= DWC3_GCTL_CORESOFTRESET; dwc3_writel(dwc->regs, DWC3_GCTL, reg); @@ -182,11 +186,11 @@ static void __dwc3_set_mode(struct work_
spin_lock_irqsave(&dwc->lock, flags);
- dwc3_set_prtcap(dwc, dwc->desired_dr_role); + dwc3_set_prtcap(dwc, desired_dr_role);
spin_unlock_irqrestore(&dwc->lock, flags);
- switch (dwc->desired_dr_role) { + switch (desired_dr_role) { case DWC3_GCTL_PRTCAP_HOST: ret = dwc3_host_init(dwc); if (ret) {