linux-kernel - Re: [PATCH v1] usb: dwc3: gadget: Prevent irq storm when TH re-executes

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] [day] [month] [year] [list]

Message-ID: <CAPTae5K7fdUPsDp-S8uOfrOrN5Pqfd91bw+UN=h65sjrTvFdzg@mail.gmail.com>
Date: Wed, 5 Feb 2025 00:32:46 -0800
From: Badhri Jagan Sridharan <badhri@...gle.com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
Cc: "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>, 
	"felipe.balbi@...ux.intel.com" <felipe.balbi@...ux.intel.com>, 
	"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>, 
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "jameswei@...gle.com" <jameswei@...gle.com>, 
	"stable@...nel.org" <stable@...nel.org>
Subject: Re: [PATCH v1] usb: dwc3: gadget: Prevent irq storm when TH re-executes

On Mon, Feb 3, 2025 at 4:22 PM Thinh Nguyen <Thinh.Nguyen@...opsys.com> wrote:
>
> On Sat, Feb 01, 2025, Badhri Jagan Sridharan wrote:
> > On Fri, Jan 31, 2025 at 4:09 PM Thinh Nguyen <Thinh.Nguyen@...opsys.com> wrote:
> > >
> > > Hmm... Can you try this instead:
> >
> > Thanks, this seems to be working !
> > I also sent out the following as there isn't a way to enable interrupt
> > moderation through the device tree node:
> > https://urldefense.com/v3/__https://lore.kernel.org/all/20250202035100.31235-1-badhri@google.com/__;!!A4F2R9G_pg!e6BA5h7M1-HZjrH2-bLVF0YbmzStu9ASv1lkZudrnyX2RFDlDUnrlFCgMdbwXcbTrMsVUQvgnpRmYxmcV-w$
> > https://urldefense.com/v3/__https://lore.kernel.org/all/20250202035100.31235-2-badhri@google.com/__;!!A4F2R9G_pg!e6BA5h7M1-HZjrH2-bLVF0YbmzStu9ASv1lkZudrnyX2RFDlDUnrlFCgMdbwXcbTrMsVUQvgnpRm9hdUtjE$
> >
> > >
> > > diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
> > > index 0fe92c0fb520..c1b5a3742ab4 100644
> > > --- a/drivers/usb/dwc3/gadget.c
> > > +++ b/drivers/usb/dwc3/gadget.c
> > > @@ -5737,14 +5737,20 @@ static irqreturn_t dwc3_process_event_buf(struct dwc3_event_buffer *evt)
> > >         dwc3_writel(dwc->regs, DWC3_GEVNTSIZ(0),
> > >                     DWC3_GEVNTSIZ_SIZE(evt->length));
> > >
> > > +       /*
> > > +        * Keep the clearing of DWC3_EVENT_PENDING after the interrupt unmask
> > > +        * but before the clearing of DWC3_GEVNTCOUNT_EHB.
> > > +        */
> > > +       evt->flags &= ~DWC3_EVENT_PENDING;
> > > +
> > > +       /* Ensure the flag is updated before clearing DWC3_GEVNTCOUNT_EHB */
> > > +       wmb();
> >
> > I still have one more question though :)
> > Wondering why not move this code about the DWC3_GEVNTSIZ write where
> > the interrupt is actually unmasked that way this would also work for
> > systems which dont have interrupt moderation enabled right ?
> >
>
> It's mainly for PCI devices. PCI devices sends assert/de-assert messages
> to represent the level interrupt for PCI legacy interrupt. The de-assert
> interrupt message may not be received right after masking the interrupt,
> such as due to system latency. This may trigger a scheduling of the
> TH again. If there's new event received during the TH, this will cause a
> race and dwc3 driver may overwrite the cached events (a window between
> clearing the flag and unmasking of interrupt).
>
> We may have a potential issue for platform with devices with shared
> interrupt too (though I haven't seen report of this issue before).

Thanks for the detailed explanation Thinh ! Appreciate that !
I am running more tests before sending out the V2 version of the patch.
Should be done in another day or two.

Thanks,
Badhri

>s
> BR,
> Thinh