linux-kernel - Re: [v2 PATCH] usb: dwc3: core: remove lock of otg mode during gadget suspend/resume to avoid deadlock

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]

Date: Tue, 18 Jun 2024 23:05:44 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Michael Nazzareno Trimarchi <michael@...rulasolutions.com>
CC: Thinh Nguyen <Thinh.Nguyen@...opsys.com>, Meng Li <Meng.Li@...driver.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "quic_uaggarwa@...cinc.com" <quic_uaggarwa@...cinc.com>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "xu.yang_2@....com" <xu.yang_2@....com>
Subject: Re: [v2 PATCH] usb: dwc3: core: remove lock of otg mode during gadget
 suspend/resume to avoid deadlock

On Wed, Jun 19, 2024, Michael Nazzareno Trimarchi wrote:
> Hi
> 
> On Wed, Jun 19, 2024 at 12:42 AM Thinh Nguyen <Thinh.Nguyen@...opsys.com> wrote:
> >
> > Hi,
> >
> > On Wed, Jun 19, 2024, Michael Nazzareno Trimarchi wrote:
> > > Hi
> > >
> > > On Tue, Jun 18, 2024 at 11:55 PM Thinh Nguyen <Thinh.Nguyen@...opsys.com> wrote:
> > > >
> > > > On Tue, Jun 18, 2024, Meng Li wrote:
> > > > > When config CONFIG_USB_DWC3_DUAL_ROLE is selected, and trigger system
> > > > > to enter suspend status with below command:
> > > > > echo mem > /sys/power/state
> > > > > There will be a deadlock issue occurring. Detailed invoking path as
> > > > > below:
> > > > > dwc3_suspend_common()
> > > > >     spin_lock_irqsave(&dwc->lock, flags);              <-- 1st
> > > > >     dwc3_gadget_suspend(dwc);
> > > > >         dwc3_gadget_soft_disconnect(dwc);
> > > > >             spin_lock_irqsave(&dwc->lock, flags);      <-- 2nd
> > >
> > >
> > > In resume path the spinlock was taken too and I see that there is a call
> > > dwc3_gadget_soft_connect that call the dwc3_core_soft_reset that can poll
> > > using msleep for time.  I don't know if this is a valid fix but seems
> > > does not fix
> > > only commit c7ebd8149ee5
> > >
> >
> > Meng did take the resume path into account. Can you clarify what you
> > meant with "does not fix only commit c7ebd8149ee5"?
> 
> I have seen that is suppose to fix something introduced by that commit
> but I think
> that resume path was already broken before
> 
> spinlock_irqsave()
> dwc3_gadget_resume -> dwc3_gadget_soft_connect -> dwc3_core_soft_reset
> (can sleep)
> 

The code path for OTG should not hit that sleep. The sleep is only
applicable to DWC_usb31 and DWC_usb32. Technically, it wasn't broken
before. But we should remove this spinlock on resume as it is done here.

BR,
Thinh