[PATCH v2] phy: qcom-qusb2: Fix NULL pointer dereference on early suspend

[Loic Poulain]

On Thu, Dec 11, 2025 at 3:18 PM Dmitry Baryshkov
<dmitry.baryshkov at oss.qualcomm.com> wrote:
>
> On Thu, 11 Dec 2025 at 16:10, Loic Poulain
> <loic.poulain at oss.qualcomm.com> wrote:
> >
> > On Thu, Dec 11, 2025 at 2:57 PM Dmitry Baryshkov
> > <dmitry.baryshkov at oss.qualcomm.com> wrote:
> > >
> > > On Thu, Dec 11, 2025 at 02:48:36PM +0100, Loic Poulain wrote:
> > > > Enabling runtime PM before attaching the QPHY instance as driver data
> > > > can lead to a NULL pointer dereference in runtime PM callbacks that
> > > > expect valid driver data. There is a small window where the suspend
> > > > callback may run after PM runtime enabling and before runtime forbid.
> > > > This causes a sporadic crash during boot:
> > > >
> > > > ```
> > > > Unable to handle kernel NULL pointer dereference at virtual address 00000000000000a1
> > > > [...]
> > > > CPU: 0 UID: 0 PID: 11 Comm: kworker/0:1 Not tainted 6.16.7+ #116 PREEMPT
> > > > Workqueue: pm pm_runtime_work
> > > > pstate: 20000005 (nzCv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
> > > > pc : qusb2_phy_runtime_suspend+0x14/0x1e0 [phy_qcom_qusb2]
> > > > lr : pm_generic_runtime_suspend+0x2c/0x44
> > > > [...]
> > > > ```
> > > >
> > > > Enable PM runtime only after attaching the QPHY instance as driver data to
> > > > avoid a NULL pointer dereference in PM runtime callbacks.
> > > >
> > > > Reorder pm_runtime_enable() and pm_runtime_forbid() to prevent a
> > > > short window where an unnecessary runtime suspend can occur.
> > > >
> > > > Use the devres-managed version to ensure PM runtime is symmetrically
> > > > disabled during driver removal for proper cleanup.
> > > >
> > > > Fixes: 891a96f65ac3 ("phy: qcom-qusb2: Add support for runtime PM")
> > > > Signed-off-by: Loic Poulain <loic.poulain at oss.qualcomm.com>
> > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov at oss.qualcomm.com>
> > > > ---
> > > >  v2: Move runtime-pm enabling after dev_set_drvdata
> > > >      use devm_pm_runtime_enable() to fix unbalanced enabling on removal
> > > >      reword commit message
> > > >
> > > >  drivers/phy/qualcomm/phy-qcom-qusb2.c | 19 ++++++++-----------
> > > >  1 file changed, 8 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/drivers/phy/qualcomm/phy-qcom-qusb2.c b/drivers/phy/qualcomm/phy-qcom-qusb2.c
> > > > index b5514a32ff8f..c496acbad0bb 100644
> > > > --- a/drivers/phy/qualcomm/phy-qcom-qusb2.c
> > > > +++ b/drivers/phy/qualcomm/phy-qcom-qusb2.c
> > > > @@ -1093,19 +1093,10 @@ static int qusb2_phy_probe(struct platform_device *pdev)
> > > >               or->hsdisc_trim.override = true;
> > > >       }
> > > >
> > > > -     pm_runtime_set_active(dev);
> > > > -     pm_runtime_enable(dev);
> > > > -     /*
> > > > -      * Prevent runtime pm from being ON by default. Users can enable
> > > > -      * it using power/control in sysfs.
> > > > -      */
> > > > -     pm_runtime_forbid(dev);
> > > > -
> > > >       generic_phy = devm_phy_create(dev, NULL, &qusb2_phy_gen_ops);
> > >
> > > No. phy_create() checks whether the device has runtime PM enabled. By
> > > moving these calls later you've disabled runtime PM for the PHY.
> >
> > Thanks for pointing this out. I can address it, but is there a
> > specific reason why the child PHY device should conditionally enable
> > its own runtime PM based on the parent’s state?
>
> Yes. It makes a life of drivers easier: if the parent has runtime PM
> enabled, enable runtime PM for the PHY device in a standard way.
> Otherwise the non-runtime-enabled PHY device will prevent the parent
> (actual device) from runtime suspending.

Ok, then I’ll move dev_set_drvdata() instead and check that runtime PM
still works, which I should have tested for this patch version...

Regards,
Loic