lkml.org 
[lkml]   [2022]   [Aug]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v2 4/9] usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup
    On Sat, Aug 06, 2022 at 06:08:51PM +0200, Johan Hovold wrote:
    > On Sat, Aug 06, 2022 at 08:03:11PM +0530, Manivannan Sadhasivam wrote:
    > > On Thu, Aug 04, 2022 at 05:09:56PM +0200, Johan Hovold wrote:
    > > > The Qualcomm dwc3 runtime-PM implementation checks the xhci
    > > > platform-device pointer in the wakeup-interrupt handler to determine
    > > > whether the controller is in host mode and if so triggers a resume.
    > > >
    > > > After a role switch in OTG mode the xhci platform-device would have been
    > > > freed and the next wakeup from runtime suspend would access the freed
    > > > memory.
    > > >
    > > > Note that role switching is executed from a freezable workqueue, which
    > > > guarantees that the pointer is stable during suspend.
    > > >
    > > > Also note that runtime PM has been broken since commit 2664deb09306
    > > > ("usb: dwc3: qcom: Honor wakeup enabled/disabled state"), which
    > > > incidentally also prevents this issue from being triggered.
    > > >
    > > > Fixes: a4333c3a6ba9 ("usb: dwc3: Add Qualcomm DWC3 glue driver")
    > > > Cc: stable@vger.kernel.org # 4.18
    > > > Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
    > >
    > > It'd be good to mention the introduction of dwc3_qcom_is_host() function.
    > > Initially I thought it is used in a single place, but going through the rest of
    > > the patches reveals that it is used later on.
    >
    > I think the helper is warranted on its own as it serves as documentation
    > of the underlying assumptions that this code relies on.
    >

    That's even better.

    Thanks,
    Mani

    > > > +/* Only usable in contexts where the role can not change. */
    > > > +static bool dwc3_qcom_is_host(struct dwc3_qcom *qcom)
    > > > +{
    > > > + struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
    > > > +
    > > > + return dwc->xhci;
    > > > +}
    > > > +
    > > > static enum usb_device_speed dwc3_qcom_read_usb2_speed(struct dwc3_qcom *qcom)
    > > > {
    > > > struct dwc3 *dwc = platform_get_drvdata(qcom->dwc3);
    > > > @@ -460,7 +468,11 @@ static irqreturn_t qcom_dwc3_resume_irq(int irq, void *data)
    > > > if (qcom->pm_suspended)
    > > > return IRQ_HANDLED;
    > > >
    > > > - if (dwc->xhci)
    > > > + /*
    > > > + * This is safe as role switching is done from a freezable workqueue
    > > > + * and the wakeup interrupts are disabled as part of resume.
    > > > + */
    > > > + if (dwc3_qcom_is_host(qcom))
    > > > pm_runtime_resume(&dwc->xhci->dev);
    > > >
    > > > return IRQ_HANDLED;
    > > > diff --git a/drivers/usb/dwc3/host.c b/drivers/usb/dwc3/host.c
    > > > index f56c30cf151e..f6f13e7f1ba1 100644
    > > > --- a/drivers/usb/dwc3/host.c
    > > > +++ b/drivers/usb/dwc3/host.c
    > > > @@ -135,4 +135,5 @@ int dwc3_host_init(struct dwc3 *dwc)
    > > > void dwc3_host_exit(struct dwc3 *dwc)
    > > > {
    > > > platform_device_unregister(dwc->xhci);
    > > > + dwc->xhci = NULL;
    > > > }
    > > > --
    > > > 2.35.1
    >
    > Johan

    --
    மணிவண்ணன் சதாசிவம்

    \
     
     \ /
      Last update: 2022-08-06 18:46    [W:5.002 / U:0.516 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site