[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20230609003217.2544eyd2sjwg6rnb@synopsys.com>
Date: Fri, 9 Jun 2023 00:32:30 +0000
From: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To: Johan Hovold <johan+linaro@...nel.org>
CC: Thinh Nguyen <Thinh.Nguyen@...opsys.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Andy Gross <agross@...nel.org>,
Bjorn Andersson <andersson@...nel.org>,
Konrad Dybcio <konrad.dybcio@...aro.org>,
Krishna Kurapati <quic_kriskura@...cinc.com>,
Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>,
"linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"stable@...r.kernel.org" <stable@...r.kernel.org>,
Sandeep Maheswaram <quic_c_sanm@...cinc.com>
Subject: Re: [PATCH 1/2] USB: dwc3: qcom: fix NULL-deref on suspend
On Wed, Jun 07, 2023, Johan Hovold wrote:
> The Qualcomm dwc3 glue driver is currently accessing the driver data of
> the child core device during suspend and on wakeup interrupts. This is
> clearly a bad idea as the child may not have probed yet or could have
> been unbound from its driver.
>
> The first such layering violation was part of the initial version of the
> driver, but this was later made worse when the hack that accesses the
> driver data of the grand child xhci device to configure the wakeup
> interrupts was added.
>
> Fixing this properly is not that easily done, so add a sanity check to
> make sure that the child driver data is non-NULL before dereferencing it
> for now.
>
> Note that this relies on subtleties like the fact that driver core is
> making sure that the parent is not suspended while the child is probing.
>
> Reported-by: Manivannan Sadhasivam <manivannan.sadhasivam@...aro.org>
> Link: https://urldefense.com/v3/__https://lore.kernel.org/all/20230325165217.31069-4-manivannan.sadhasivam@linaro.org/__;!!A4F2R9G_pg!dk6pg2XJjKSKBmzlobQwPOXDen4GHR-wW53sBe71g0X6gbC7AfOePhrb76oTLN6yHWTvKBy_y5pwSE1_HTASMhOnd5zz$
> Fixes: d9152161b4bf ("usb: dwc3: Add Qualcomm DWC3 glue layer driver")
> Fixes: 6895ea55c385 ("usb: dwc3: qcom: Configure wakeup interrupts during suspend")
> Cc: stable@...r.kernel.org # 3.18: a872ab303d5d: "usb: dwc3: qcom: fix use-after-free on runtime-PM wakeup"
> Cc: Sandeep Maheswaram <quic_c_sanm@...cinc.com>
> Cc: Krishna Kurapati <quic_kriskura@...cinc.com>
> Signed-off-by: Johan Hovold <johan+linaro@...nel.org>
> ---
> drivers/usb/dwc3/dwc3-qcom.c | 11 ++++++++++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/dwc3/dwc3-qcom.c b/drivers/usb/dwc3/dwc3-qcom.c
> index 959fc925ca7c..79b22abf9727 100644
> --- a/drivers/usb/dwc3/dwc3-qcom.c
> +++ b/drivers/usb/dwc3/dwc3-qcom.c
> @@ -308,7 +308,16 @@ static void dwc3_qcom_interconnect_exit(struct dwc3_qcom *qcom)
> /* 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);
> + struct dwc3 *dwc;
> +
> + /*
> + * FIXME: Fix this layering violation.
> + */
> + dwc = platform_get_drvdata(qcom->dwc3);
> +
> + /* Core driver may not have probed yet. */
> + if (!dwc)
> + return false;
>
> return dwc->xhci;
> }
> --
> 2.39.3
>
Thanks for the catch.
Acked-by: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
BR,
Thinh
Powered by blists - more mailing lists