[PATCH v3 0/5] phy: qcom: Fix possible NULL-deref and runtime PM race conditions

[PATCH v3 0/5] phy: qcom: Fix possible NULL-deref and runtime PM race conditions

[Loic Poulain]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=y, Size: 1532 bytes --]

Address potential NULL pointer dereferences and race conditions related
to runtime PM in several Qualcomm PHY drivers. In all cases, enabling
runtime PM before the PHY instance is fully initialized can lead to
crashes during early runtime suspend callbacks.

- Attach driver data before enabling runtime PM.
- Introduce initialization flags where needed to avoid dereferencing
uninitialized pointers.
- Reorder pm_runtime_enable() and pm_runtime_forbid() calls to prevent
unnecessary suspend/resume cycles during driver probe.
- Use devres-managed PM runtime helpers for proper cleanup.

Changes in V3:
Rebase on next and remove 2/6 (obsolete)

Changes in V2:
Split patches 2/4 and 3/4 so that the null‑pointer dereference fix and
the runtime‑PM enable/forbid reordering are logically separated.

Loic Poulain (5):
  phy: qcom: qmp-combo: Prevent unnecessary PM runtime suspend at boot
  phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot
  phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime
    suspend
  phy: qcom: qmp-usb-legacy: Prevent unnecessary PM runtime suspend at
    boot
  phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime
    suspend

 drivers/phy/qualcomm/phy-qcom-qmp-combo.c     | 10 ++++-----
 .../phy/qualcomm/phy-qcom-qmp-usb-legacy.c    | 21 ++++++++++++-------
 drivers/phy/qualcomm/phy-qcom-qmp-usbc.c      | 10 ++++-----
 drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 15 ++++++-------
 4 files changed, 32 insertions(+), 24 deletions(-)

-- 
2.34.1



[-- Attachment #2: Type: text/plain, Size: 112 bytes --]

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v3 1/5] phy: qcom: qmp-combo: Prevent unnecessary PM runtime suspend at boot

[Loic Poulain]

There is a small window where the device can suspend after
pm_runtime_enable() and before pm_runtime_forbid(), causing an
unnecessary suspend/resume cycle while the PHY is not yet registered.

Move pm_runtime_forbid() before pm_runtime_enable() to eliminate
this race.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-combo.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index 93f1aa10d400..c3661872bb7a 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -4932,15 +4932,15 @@ static int qmp_combo_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_node_put;
 
+	/*
+	 * Enable runtime PM support, but forbid it by default.
+	 * Users can allow it again via the power/control attribute in sysfs.
+	 */
 	pm_runtime_set_active(dev);
+	pm_runtime_forbid(dev);
 	ret = devm_pm_runtime_enable(dev);
 	if (ret)
 		goto err_node_put;
-	/*
-	 * Prevent runtime pm from being ON by default. Users can enable
-	 * it using power/control in sysfs.
-	 */
-	pm_runtime_forbid(dev);
 
 	ret = qmp_combo_register_clocks(qmp, usb_np, dp_np);
 	if (ret)
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v3 1/5] phy: qcom: qmp-combo: Prevent unnecessary PM runtime suspend at boot

[Johan Hovold]

On Thu, Feb 05, 2026 at 05:02:36PM +0100, Loic Poulain wrote:
> There is a small window where the device can suspend after
> pm_runtime_enable() and before pm_runtime_forbid(), causing an
> unnecessary suspend/resume cycle while the PHY is not yet registered.

What do you think can trigger a suspend in that window?

(A racing user space request to both disable and re-enable runtime PM
could theoretically do so but not in practice.).

> Move pm_runtime_forbid() before pm_runtime_enable() to eliminate
> this race.

I think the commit message should reflect that this isn't really an
issue.

Johan

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v3 2/5] phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot

[Loic Poulain]

There is a small window where the device can suspend after
pm_runtime_enable() and before pm_runtime_forbid(), causing an
unnecessary suspend/resume cycle while the PHY is not yet registered.

Move pm_runtime_forbid() before pm_runtime_enable() to eliminate
this race.

Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
index 14feb77789b3..90ea6ca64026 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
@@ -1959,15 +1959,15 @@ static int qmp_usbc_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_node_put;
 
+	/*
+	 * Enable runtime PM support, but forbid it by default.
+	 * Users can allow it again via the power/control attribute in sysfs.
+	 */
 	pm_runtime_set_active(dev);
+	pm_runtime_forbid(dev);
 	ret = devm_pm_runtime_enable(dev);
 	if (ret)
 		goto err_node_put;
-	/*
-	 * Prevent runtime pm from being ON by default. Users can enable
-	 * it using power/control in sysfs.
-	 */
-	pm_runtime_forbid(dev);
 
 	ret = qmp_usbc_register_clocks(qmp, np);
 	if (ret)
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v3 2/5] phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot

[Dmitry Baryshkov]

On Thu, Feb 05, 2026 at 05:02:37PM +0100, Loic Poulain wrote:
> There is a small window where the device can suspend after
> pm_runtime_enable() and before pm_runtime_forbid(), causing an
> unnecessary suspend/resume cycle while the PHY is not yet registered.
> 
> Move pm_runtime_forbid() before pm_runtime_enable() to eliminate
> this race.
> 
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-usbc.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v3 3/5] phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend

[Loic Poulain]

There is a small window where the runtime suspend callback may run
after pm_runtime_enable() and before pm_runtime_forbid(). In this
case, a crash occurs because runtime suspend/resume dereferences
qmp->phy pointer, which is not yet initialized:
        `if (!qmp->phy->init_count) {`

This can also happen if user re-enables runtime-pm via the sysfs
attribute before qmp phy is initialized.

Similarly to other qcom phy drivers, introduce a qmp->phy_initialized
variable that can be used to avoid relying on the possibly uninitialized
phy pointer.

Fixes: e464a3180a43 ("phy: qcom-qmp-usb: split off the legacy USB+dp_com support")
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
index 8bf951b0490c..258e0e966a02 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
@@ -541,6 +541,7 @@ struct qmp_usb {
 	struct regulator_bulk_data *vregs;
 
 	enum phy_mode mode;
+	bool phy_initialized;
 
 	struct phy *phy;
 
@@ -895,6 +896,7 @@ static int qmp_usb_legacy_power_off(struct phy *phy)
 
 static int qmp_usb_legacy_enable(struct phy *phy)
 {
+	struct qmp_usb *qmp = phy_get_drvdata(phy);
 	int ret;
 
 	ret = qmp_usb_legacy_init(phy);
@@ -904,14 +906,19 @@ static int qmp_usb_legacy_enable(struct phy *phy)
 	ret = qmp_usb_legacy_power_on(phy);
 	if (ret)
 		qmp_usb_legacy_exit(phy);
+	else
+		qmp->phy_initialized = true;
 
 	return ret;
 }
 
 static int qmp_usb_legacy_disable(struct phy *phy)
 {
+	struct qmp_usb *qmp = phy_get_drvdata(phy);
 	int ret;
 
+	qmp->phy_initialized = false;
+
 	ret = qmp_usb_legacy_power_off(phy);
 	if (ret)
 		return ret;
@@ -988,7 +995,7 @@ static int __maybe_unused qmp_usb_legacy_runtime_suspend(struct device *dev)
 
 	dev_vdbg(dev, "Suspending QMP phy, mode:%d\n", qmp->mode);
 
-	if (!qmp->phy->init_count) {
+	if (!qmp->phy_initialized) {
 		dev_vdbg(dev, "PHY not initialized, bailing out\n");
 		return 0;
 	}
@@ -1009,7 +1016,7 @@ static int __maybe_unused qmp_usb_legacy_runtime_resume(struct device *dev)
 
 	dev_vdbg(dev, "Resuming QMP phy, mode:%d\n", qmp->mode);
 
-	if (!qmp->phy->init_count) {
+	if (!qmp->phy_initialized) {
 		dev_vdbg(dev, "PHY not initialized, bailing out\n");
 		return 0;
 	}
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v3 3/5] phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend

[Dmitry Baryshkov]

On Thu, Feb 05, 2026 at 05:02:38PM +0100, Loic Poulain wrote:
> There is a small window where the runtime suspend callback may run
> after pm_runtime_enable() and before pm_runtime_forbid(). In this
> case, a crash occurs because runtime suspend/resume dereferences
> qmp->phy pointer, which is not yet initialized:
>         `if (!qmp->phy->init_count) {`
> 
> This can also happen if user re-enables runtime-pm via the sysfs
> attribute before qmp phy is initialized.
> 
> Similarly to other qcom phy drivers, introduce a qmp->phy_initialized
> variable that can be used to avoid relying on the possibly uninitialized
> phy pointer.
> 
> Fixes: e464a3180a43 ("phy: qcom-qmp-usb: split off the legacy USB+dp_com support")
> Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v3 3/5] phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend

[Johan Hovold]

On Thu, Feb 05, 2026 at 05:02:38PM +0100, Loic Poulain wrote:
> There is a small window where the runtime suspend callback may run
> after pm_runtime_enable() and before pm_runtime_forbid(). In this
> case, a crash occurs because runtime suspend/resume dereferences
> qmp->phy pointer, which is not yet initialized:
>         `if (!qmp->phy->init_count) {`

So here too, what would trigger a suspend in this window? (Except
possibly user space disabling and reenabling runtime pm, which can't
happen in practice).

> This can also happen if user re-enables runtime-pm via the sysfs
> attribute before qmp phy is initialized.

This I guess can happen in theory, but you'd need to try pretty hard.

But I think the commit message should better reflect this is all mostly
theoretical (currently it sounds like something you've actually hit).

Johan

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v3 4/5] phy: qcom: qmp-usb-legacy: Prevent unnecessary PM runtime suspend at boot

[Loic Poulain]

There is a small window where the device can suspend after
pm_runtime_enable() and before pm_runtime_forbid(), causing an
unnecessary suspend/resume cycle while the PHY is not yet registered.

Move pm_runtime_forbid() before pm_runtime_enable() to eliminate
this race.

Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
index 258e0e966a02..73439d223f1d 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
@@ -1284,15 +1284,15 @@ static int qmp_usb_legacy_probe(struct platform_device *pdev)
 	if (ret)
 		goto err_node_put;
 
+	/*
+	 * Enable runtime PM support, but forbid it by default.
+	 * Users can allow it again via the power/control attribute in sysfs.
+	 */
 	pm_runtime_set_active(dev);
+	pm_runtime_forbid(dev);
 	ret = devm_pm_runtime_enable(dev);
 	if (ret)
 		goto err_node_put;
-	/*
-	 * Prevent runtime pm from being ON by default. Users can enable
-	 * it using power/control in sysfs.
-	 */
-	pm_runtime_forbid(dev);
 
 	ret = phy_pipe_clk_register(qmp, np);
 	if (ret)
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v3 4/5] phy: qcom: qmp-usb-legacy: Prevent unnecessary PM runtime suspend at boot

[Dmitry Baryshkov]

On Thu, Feb 05, 2026 at 05:02:39PM +0100, Loic Poulain wrote:
> There is a small window where the device can suspend after
> pm_runtime_enable() and before pm_runtime_forbid(), causing an
> unnecessary suspend/resume cycle while the PHY is not yet registered.
> 
> Move pm_runtime_forbid() before pm_runtime_enable() to eliminate
> this race.
> 
> Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
> ---
>  drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>


-- 
With best wishes
Dmitry

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

[PATCH v3 5/5] phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend

[Loic Poulain]

Enabling runtime PM before attaching the hsphy 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.

Attach the hsphy instance as driver data before enabling runtime PM to
prevent NULL pointer dereference in runtime PM 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: 0d75f508a9d5 ("phy: qcom-snps: Add runtime suspend and resume handlers")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
---
 drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
index eb0b0f61d98e..d1288a6c202e 100644
--- a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
+++ b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
@@ -599,13 +599,17 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
 		return dev_err_probe(dev, ret,
 				     "failed to get regulator supplies\n");
 
-	pm_runtime_set_active(dev);
-	pm_runtime_enable(dev);
+	dev_set_drvdata(dev, hsphy);
+
 	/*
-	 * Prevent runtime pm from being ON by default. Users can enable
-	 * it using power/control in sysfs.
+	 * Enable runtime PM support, but forbid it by default.
+	 * Users can allow it again via the power/control attribute in sysfs.
 	 */
+	pm_runtime_set_active(dev);
 	pm_runtime_forbid(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret)
+		return ret;
 
 	generic_phy = devm_phy_create(dev, NULL, &qcom_snps_hsphy_gen_ops);
 	if (IS_ERR(generic_phy)) {
@@ -615,15 +619,12 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
 	}
 	hsphy->phy = generic_phy;
 
-	dev_set_drvdata(dev, hsphy);
 	phy_set_drvdata(generic_phy, hsphy);
 	qcom_snps_hsphy_read_override_param_seq(dev);
 
 	phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
 	if (!IS_ERR(phy_provider))
 		dev_dbg(dev, "Registered Qcom-SNPS HS phy\n");
-	else
-		pm_runtime_disable(dev);
 
 	return PTR_ERR_OR_ZERO(phy_provider);
 }
-- 
2.34.1


-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v3 5/5] phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend

[Johan Hovold]

On Thu, Feb 05, 2026 at 05:02:40PM +0100, Loic Poulain wrote:
> Enabling runtime PM before attaching the hsphy 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.

So here too, the commit should reflect that this cannot really happen in
practice.

> Attach the hsphy instance as driver data before enabling runtime PM to
> prevent NULL pointer dereference in runtime PM 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: 0d75f508a9d5 ("phy: qcom-snps: Add runtime suspend and resume handlers")
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
> Reviewed-by: Abel Vesa <abel.vesa@oss.qualcomm.com>
> Signed-off-by: Loic Poulain <loic.poulain@oss.qualcomm.com>

Johan

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v3 5/5] phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend

[Loic Poulain]

Hi Johan,

On Fri, Feb 13, 2026 at 10:07 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Thu, Feb 05, 2026 at 05:02:40PM +0100, Loic Poulain wrote:
> > Enabling runtime PM before attaching the hsphy 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.
>
> So here too, the commit should reflect that this cannot really happen in
> practice.

This happened  in practice in the qcom‑qusb2 PHY driver, with the same
code flow.
Bug: https://github.com/qualcomm-linux/qcom-deb-images/issues/208
Patch: https://lore.kernel.org/linux-arm-msm/20251219085640.114473-1-loic.poulain@oss.qualcomm.com/

I know it may sound unlikely, but this crash has been reported
several times during boot‑stress testing. I haven’t investigated
deeply enough to determine whether it’s caused by an unfortunate
preemption window or a racing CPU.

I thought the series was already fairly conservative in its wording.
The titles use terms like “possible” and “unnecessary” to qualify the
crashes or unintended events. I can switch to “unlikely” if that
better characterizes the situation, but the issue isn’t purely
hypothetical.

Regards,
Loic

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v3 5/5] phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend

[Johan Hovold]

On Fri, Feb 13, 2026 at 10:45:32AM +0100, Loic Poulain wrote:
> On Fri, Feb 13, 2026 at 10:07 AM Johan Hovold <johan@kernel.org> wrote:
> >
> > On Thu, Feb 05, 2026 at 05:02:40PM +0100, Loic Poulain wrote:
> > > Enabling runtime PM before attaching the hsphy 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.
> >
> > So here too, the commit should reflect that this cannot really happen in
> > practice.
> 
> This happened  in practice in the qcom‑qusb2 PHY driver, with the same
> code flow.
> Bug: https://github.com/qualcomm-linux/qcom-deb-images/issues/208
> Patch: https://lore.kernel.org/linux-arm-msm/20251219085640.114473-1-loic.poulain@oss.qualcomm.com/

Thanks for the link.

> I know it may sound unlikely, but this crash has been reported
> several times during boot‑stress testing. I haven’t investigated
> deeply enough to determine whether it’s caused by an unfortunate
> preemption window or a racing CPU.

But I'm literally asking for *what* would trigger the suspend in that
initial window between enable() and forbid() cause I don't see it.

A racing user space daemon re-enabling runtime PM after forbid() is
the only thing I can think of that could trigger this.

Johan

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v3 5/5] phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend

[Loic Poulain]

On Fri, Feb 13, 2026 at 11:45 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Fri, Feb 13, 2026 at 10:45:32AM +0100, Loic Poulain wrote:
> > On Fri, Feb 13, 2026 at 10:07 AM Johan Hovold <johan@kernel.org> wrote:
> > >
> > > On Thu, Feb 05, 2026 at 05:02:40PM +0100, Loic Poulain wrote:
> > > > Enabling runtime PM before attaching the hsphy 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.
> > >
> > > So here too, the commit should reflect that this cannot really happen in
> > > practice.
> >
> > This happened  in practice in the qcom‑qusb2 PHY driver, with the same
> > code flow.
> > Bug: https://github.com/qualcomm-linux/qcom-deb-images/issues/208
> > Patch: https://lore.kernel.org/linux-arm-msm/20251219085640.114473-1-loic.poulain@oss.qualcomm.com/
>
> Thanks for the link.
>
> > I know it may sound unlikely, but this crash has been reported
> > several times during boot‑stress testing. I haven’t investigated
> > deeply enough to determine whether it’s caused by an unfortunate
> > preemption window or a racing CPU.
>
> But I'm literally asking for *what* would trigger the suspend in that
> initial window between enable() and forbid() cause I don't see it.

To be honest, I had not initially looked into the exact cause of the
suspend trigger until now, but here is what is happening.

The PHY is a supplier of the USB device. A USB device cannot be probed
until all its suppliers are ready. As long as the PHY is not ready, the
device core keeps retrying the probe, which fails with -EPROBE_DEFER.

At some point the PHY probe finally runs, but the device core may still be
attempting to probe the USB device concurrently.

Inside __driver_probe_device(), we have:

    ret = really_probe(dev, drv);
    pm_request_idle(dev);

    if (dev->parent)
        pm_runtime_put(dev->parent);

    pm_runtime_put_suppliers(dev);
    return ret;

This means that whenever a USB probe attempt completes, whether with an
error or not, its suppliers are released via pm_runtime_put_suppliers().
Releasing suppliers may in turn trigger a runtime suspend.

In our case, since the PHY is a supplier of the USB device, the USB core
keeps 'looping' in __driver_probe_device() returning -EPROBE_DEFER until
the PHY becomes ready. As a result, pm_runtime_put_suppliers() may run
concurrently with the PHY's probe function. If this happens after
runtime PM has been enabled for the PHY, but before the driver has
forbidden suspend or taken a PM reference, the PHY may end up being
runtime-suspended 'unexpectedly'.

Regards,
Loic

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v3 5/5] phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend

[Vladimir Oltean]

Hello Loic,

On Fri, Feb 13, 2026 at 04:04:43PM +0100, Loic Poulain wrote:
> On Fri, Feb 13, 2026 at 11:45 AM Johan Hovold <johan@kernel.org> wrote:
> >
> > On Fri, Feb 13, 2026 at 10:45:32AM +0100, Loic Poulain wrote:
> > > On Fri, Feb 13, 2026 at 10:07 AM Johan Hovold <johan@kernel.org> wrote:
> > > >
> > > > On Thu, Feb 05, 2026 at 05:02:40PM +0100, Loic Poulain wrote:
> > > > > Enabling runtime PM before attaching the hsphy 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.
> > > >
> > > > So here too, the commit should reflect that this cannot really happen in
> > > > practice.
> > >
> > > This happened  in practice in the qcom‑qusb2 PHY driver, with the same
> > > code flow.
> > > Bug: https://github.com/qualcomm-linux/qcom-deb-images/issues/208
> > > Patch: https://lore.kernel.org/linux-arm-msm/20251219085640.114473-1-loic.poulain@oss.qualcomm.com/
> >
> > Thanks for the link.
> >
> > > I know it may sound unlikely, but this crash has been reported
> > > several times during boot‑stress testing. I haven’t investigated
> > > deeply enough to determine whether it’s caused by an unfortunate
> > > preemption window or a racing CPU.
> >
> > But I'm literally asking for *what* would trigger the suspend in that
> > initial window between enable() and forbid() cause I don't see it.
> 
> To be honest, I had not initially looked into the exact cause of the
> suspend trigger until now, but here is what is happening.
> 
> The PHY is a supplier of the USB device. A USB device cannot be probed
> until all its suppliers are ready. As long as the PHY is not ready, the
> device core keeps retrying the probe, which fails with -EPROBE_DEFER.
> 
> At some point the PHY probe finally runs, but the device core may still be
> attempting to probe the USB device concurrently.
> 
> Inside __driver_probe_device(), we have:
> 
>     ret = really_probe(dev, drv);
>     pm_request_idle(dev);
> 
>     if (dev->parent)
>         pm_runtime_put(dev->parent);
> 
>     pm_runtime_put_suppliers(dev);
>     return ret;
> 
> This means that whenever a USB probe attempt completes, whether with an
> error or not, its suppliers are released via pm_runtime_put_suppliers().
> Releasing suppliers may in turn trigger a runtime suspend.
> 
> In our case, since the PHY is a supplier of the USB device, the USB core
> keeps 'looping' in __driver_probe_device() returning -EPROBE_DEFER until
> the PHY becomes ready. As a result, pm_runtime_put_suppliers() may run
> concurrently with the PHY's probe function. If this happens after
> runtime PM has been enabled for the PHY, but before the driver has
> forbidden suspend or taken a PM reference, the PHY may end up being
> runtime-suspended 'unexpectedly'.

Please resend this patch with the commit message including this
explanation (note that your code snippet from __driver_probe_device() is
missing a relevant call to pm_runtime_get_suppliers()).

Also, please separate the devres change to its own patch. It is fixing a
different logical issue (missing pm_runtime_disable() causes device with
unbound driver to have non-zero dev->power.disable_depth; should warn on
driver re-probe).

Another comment upon reviewing this driver's runtime PM use (although
this is at most something that may result in a patch for "next"):

This driver uses hsphy->phy_initialized to make sure qcom_snps_hsphy_suspend()
isn't called unless qcom_snps_hsphy_init() was called.

Don't we achieve the same behaviour by replacing "hsphy->phy_initialized = true"
with pm_runtime_get_sync(dev) and "hsphy->phy_initialized = false" with
pm_runtime_put(dev)?

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v3 5/5] phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend

[Johan Hovold]

On Fri, Feb 13, 2026 at 10:15:50PM +0200, Vladimir Oltean wrote:

> Another comment upon reviewing this driver's runtime PM use (although
> this is at most something that may result in a patch for "next"):
> 
> This driver uses hsphy->phy_initialized to make sure qcom_snps_hsphy_suspend()
> isn't called unless qcom_snps_hsphy_init() was called.
> 
> Don't we achieve the same behaviour by replacing "hsphy->phy_initialized = true"
> with pm_runtime_get_sync(dev) and "hsphy->phy_initialized = false" with
> pm_runtime_put(dev)?

No, the device can still suspend before phy_init() is called.

What would work, and which should probably be preferred over adding
these phy_initialized flags, is to increment the pm usage counter before
enabling runtime pm and decrementing it after the PHY has been created.

Johan

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v3 5/5] phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend

[Loic Poulain]

On Mon, Feb 16, 2026 at 11:48 AM Johan Hovold <johan@kernel.org> wrote:
>
> On Fri, Feb 13, 2026 at 10:15:50PM +0200, Vladimir Oltean wrote:
>
> > Another comment upon reviewing this driver's runtime PM use (although
> > this is at most something that may result in a patch for "next"):
> >
> > This driver uses hsphy->phy_initialized to make sure qcom_snps_hsphy_suspend()
> > isn't called unless qcom_snps_hsphy_init() was called.
> >
> > Don't we achieve the same behaviour by replacing "hsphy->phy_initialized = true"
> > with pm_runtime_get_sync(dev) and "hsphy->phy_initialized = false" with
> > pm_runtime_put(dev)?
>
> No, the device can still suspend before phy_init() is called.
>
> What would work, and which should probably be preferred over adding
> these phy_initialized flags, is to increment the pm usage counter before
> enabling runtime pm and decrementing it after the PHY has been created.

Ok, yes, using the usual pm_runtime_get_noresume() before enabling
runtime PM would work as well. This is conceptually similar to this
change, which instead relies on pm_runtime_forbid(). However, I agree
that forbid provides no guarantee about when runtime PM may be
re-enabled by a user...

Regards,
Loic

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy

Re: [PATCH v3 5/5] phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend

[Johan Hovold]

On Fri, Feb 13, 2026 at 04:04:43PM +0100, Loic Poulain wrote:
> On Fri, Feb 13, 2026 at 11:45 AM Johan Hovold <johan@kernel.org> wrote:

> > But I'm literally asking for *what* would trigger the suspend in that
> > initial window between enable() and forbid() cause I don't see it.
> 
> To be honest, I had not initially looked into the exact cause of the
> suspend trigger until now, but here is what is happening.
> 
> The PHY is a supplier of the USB device. A USB device cannot be probed
> until all its suppliers are ready. As long as the PHY is not ready, the
> device core keeps retrying the probe, which fails with -EPROBE_DEFER.
> 
> At some point the PHY probe finally runs, but the device core may still be
> attempting to probe the USB device concurrently.
> 
> Inside __driver_probe_device(), we have:
> 
>     ret = really_probe(dev, drv);
>     pm_request_idle(dev);
> 
>     if (dev->parent)
>         pm_runtime_put(dev->parent);
> 
>     pm_runtime_put_suppliers(dev);
>     return ret;
> 
> This means that whenever a USB probe attempt completes, whether with an
> error or not, its suppliers are released via pm_runtime_put_suppliers().
> Releasing suppliers may in turn trigger a runtime suspend.
> 
> In our case, since the PHY is a supplier of the USB device, the USB core
> keeps 'looping' in __driver_probe_device() returning -EPROBE_DEFER until
> the PHY becomes ready. As a result, pm_runtime_put_suppliers() may run
> concurrently with the PHY's probe function. If this happens after
> runtime PM has been enabled for the PHY, but before the driver has
> forbidden suspend or taken a PM reference, the PHY may end up being
> runtime-suspended 'unexpectedly'.

Thanks for tracking that down. That's an unexpected side effect of
fw_devlink adding runtime pm enabled links (which is the default
behaviour since late 2023).

Johan

-- 
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy