* [PATCH v4 1/4] phy: qcom: qmp-combo: Prevent unnecessary PM runtime suspend at boot
2026-07-20 14:53 [PATCH v4 0/4] phy: qcom: Fix possible NULL-deref and runtime PM race conditions Loic Poulain
@ 2026-07-20 14:53 ` Loic Poulain
2026-07-20 14:53 ` [PATCH v4 2/4] phy: qcom: qmp-usbc: " Loic Poulain
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Loic Poulain @ 2026-07-20 14:53 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng
Cc: linux-arm-msm, linux-phy, linux-kernel, Dmitry Baryshkov,
Abel Vesa, Konrad Dybcio, Loic Poulain
Runtime PM has to be enabled before creating the PHYs, since phy_create()
only enables runtime PM on the PHY devices if it is already enabled on
this parent device. This opens a small window where the device can be
runtime suspended after pm_runtime_enable() and before the later
pm_runtime_forbid(), causing an unnecessary suspend/resume cycle while
the PHYs are not yet registered.
Take a runtime PM usage reference with pm_runtime_get_noresume() before
enabling runtime PM and release it once the PHYs have been created to
prevent the device from being runtime suspended during that window.
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 | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
index cdcfad2e86b1d37650c4d7b0433319837ee9c9d4..affc8e040bb968a5b9d0b6f6fc2694c471368729 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-combo.c
@@ -4934,10 +4934,16 @@ static int qmp_combo_probe(struct platform_device *pdev)
if (ret)
goto err_node_put;
+ /*
+ * Enable runtime PM before creating the PHYs, phy_create() only enables
+ * it on the PHY devices if already enabled on the parent. Hold a usage
+ * reference so callbacks cannot run until the PHY is ready.
+ */
+ pm_runtime_get_noresume(dev);
pm_runtime_set_active(dev);
ret = devm_pm_runtime_enable(dev);
if (ret)
- goto err_node_put;
+ goto err_pm_put;
/*
* Prevent runtime pm from being ON by default. Users can enable
* it using power/control in sysfs.
@@ -4946,14 +4952,13 @@ static int qmp_combo_probe(struct platform_device *pdev)
ret = qmp_combo_register_clocks(qmp, usb_np, dp_np);
if (ret)
- goto err_node_put;
-
+ goto err_pm_put;
qmp->usb_phy = devm_phy_create(dev, usb_np, &qmp_combo_usb_phy_ops);
if (IS_ERR(qmp->usb_phy)) {
ret = PTR_ERR(qmp->usb_phy);
dev_err(dev, "failed to create USB PHY: %d\n", ret);
- goto err_node_put;
+ goto err_pm_put;
}
phy_set_drvdata(qmp->usb_phy, qmp);
@@ -4962,11 +4967,13 @@ static int qmp_combo_probe(struct platform_device *pdev)
if (IS_ERR(qmp->dp_phy)) {
ret = PTR_ERR(qmp->dp_phy);
dev_err(dev, "failed to create DP PHY: %d\n", ret);
- goto err_node_put;
+ goto err_pm_put;
}
phy_set_drvdata(qmp->dp_phy, qmp);
+ pm_runtime_put(dev);
+
if (usb_np == dev->of_node)
phy_provider = devm_of_phy_provider_register(dev, qmp_combo_phy_xlate);
else
@@ -4977,6 +4984,8 @@ static int qmp_combo_probe(struct platform_device *pdev)
return PTR_ERR_OR_ZERO(phy_provider);
+err_pm_put:
+ pm_runtime_put_noidle(dev);
err_node_put:
of_node_put(usb_np);
of_node_put(dp_np);
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v4 2/4] phy: qcom: qmp-usbc: Prevent unnecessary PM runtime suspend at boot
2026-07-20 14:53 [PATCH v4 0/4] phy: qcom: Fix possible NULL-deref and runtime PM race conditions Loic Poulain
2026-07-20 14:53 ` [PATCH v4 1/4] phy: qcom: qmp-combo: Prevent unnecessary PM runtime suspend at boot Loic Poulain
@ 2026-07-20 14:53 ` Loic Poulain
2026-07-20 14:53 ` [PATCH v4 3/4] phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend Loic Poulain
2026-07-20 14:53 ` [PATCH v4 4/4] phy: qcom: snps-femto-v2: " Loic Poulain
3 siblings, 0 replies; 6+ messages in thread
From: Loic Poulain @ 2026-07-20 14:53 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng
Cc: linux-arm-msm, linux-phy, linux-kernel, Loic Poulain,
Konrad Dybcio, Abel Vesa
Runtime PM has to be enabled before creating the PHYs, since phy_create()
only enables runtime PM on the PHY devices if it is already enabled on
this parent device. This opens a small window where the device can be
runtime suspended after pm_runtime_enable() and before the later
pm_runtime_forbid(), causing an unnecessary suspend/resume cycle while
the PHYs are not yet registered.
Take a runtime PM usage reference with pm_runtime_get_noresume() before
enabling runtime PM and release it once the PHYs have been created to
prevent the device from being runtime suspended during that window.
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 | 18 ++++++++++++++----
1 file changed, 14 insertions(+), 4 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
index ab3055bb5b0c198832ae06dfcd04fd34395e271d..4317224070fd8dec98090e9200ac7d0e97265979 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-usbc.c
@@ -1959,10 +1959,16 @@ static int qmp_usbc_probe(struct platform_device *pdev)
if (ret)
goto err_node_put;
+ /*
+ * Enable runtime PM before creating the PHYs, phy_create() only enables
+ * it on the PHY devices if already enabled on the parent. Hold a usage
+ * reference so callbacks cannot run until the PHY is ready.
+ */
+ pm_runtime_get_noresume(dev);
pm_runtime_set_active(dev);
ret = devm_pm_runtime_enable(dev);
if (ret)
- goto err_node_put;
+ goto err_pm_put;
/*
* Prevent runtime pm from being ON by default. Users can enable
* it using power/control in sysfs.
@@ -1971,13 +1977,13 @@ static int qmp_usbc_probe(struct platform_device *pdev)
ret = qmp_usbc_register_clocks(qmp, np);
if (ret)
- goto err_node_put;
+ goto err_pm_put;
qmp->usb_phy = devm_phy_create(dev, np, &qmp_usbc_usb_phy_ops);
if (IS_ERR(qmp->usb_phy)) {
ret = PTR_ERR(qmp->usb_phy);
dev_err(dev, "failed to create PHY: %d\n", ret);
- goto err_node_put;
+ goto err_pm_put;
}
phy_set_drvdata(qmp->usb_phy, qmp);
@@ -1987,17 +1993,21 @@ static int qmp_usbc_probe(struct platform_device *pdev)
if (IS_ERR(qmp->dp_phy)) {
ret = PTR_ERR(qmp->dp_phy);
dev_err(dev, "failed to create PHY: %d\n", ret);
- goto err_node_put;
+ goto err_pm_put;
}
phy_set_drvdata(qmp->dp_phy, qmp);
}
+ pm_runtime_put(dev);
+
of_node_put(np);
phy_provider = devm_of_phy_provider_register(dev, qmp_usbc_phy_xlate);
return PTR_ERR_OR_ZERO(phy_provider);
+err_pm_put:
+ pm_runtime_put_noidle(dev);
err_node_put:
of_node_put(np);
return ret;
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v4 3/4] phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend
2026-07-20 14:53 [PATCH v4 0/4] phy: qcom: Fix possible NULL-deref and runtime PM race conditions Loic Poulain
2026-07-20 14:53 ` [PATCH v4 1/4] phy: qcom: qmp-combo: Prevent unnecessary PM runtime suspend at boot Loic Poulain
2026-07-20 14:53 ` [PATCH v4 2/4] phy: qcom: qmp-usbc: " Loic Poulain
@ 2026-07-20 14:53 ` Loic Poulain
2026-07-20 15:10 ` sashiko-bot
2026-07-20 14:53 ` [PATCH v4 4/4] phy: qcom: snps-femto-v2: " Loic Poulain
3 siblings, 1 reply; 6+ messages in thread
From: Loic Poulain @ 2026-07-20 14:53 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng
Cc: linux-arm-msm, linux-phy, linux-kernel, Abel Vesa, Loic Poulain
Runtime PM has to be enabled before creating the PHY, since phy_create()
only enables runtime PM on the PHY device if it is already enabled on
this parent device. However, the runtime suspend/resume callbacks
dereference the qmp->phy pointer, which is only assigned by
devm_phy_create(), later in probe:
`if (!qmp->phy->init_count) {`
There is thus a small window where the callback may run after
pm_runtime_enable() but before qmp->phy is set, dereferencing a NULL
pointer. This can also happen if user re-enables runtime-pm via the
sysfs attribute before qmp phy is initialized.
Take a runtime PM usage reference with pm_runtime_get_noresume() before
enabling runtime PM and release it once qmp->phy has been created, so
that no runtime suspend can run while the phy pointer is still
uninitialized.
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 | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
index 8bf951b0490cfd811635df8940de1b789e21b46c..8b91eec0b5eee0c76b64e2c2b7a15f0169ce4b09 100644
--- a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
+++ b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
@@ -1277,10 +1277,16 @@ static int qmp_usb_legacy_probe(struct platform_device *pdev)
if (ret)
goto err_node_put;
+ /*
+ * Enable runtime PM before creating the PHY, phy_create() only enables
+ * it on the PHY device if already enabled on the parent. Hold a usage
+ * reference so callbacks cannot run before qmp->phy is assigned.
+ */
+ pm_runtime_get_noresume(dev);
pm_runtime_set_active(dev);
ret = devm_pm_runtime_enable(dev);
if (ret)
- goto err_node_put;
+ goto err_pm_put;
/*
* Prevent runtime pm from being ON by default. Users can enable
* it using power/control in sysfs.
@@ -1289,23 +1295,27 @@ static int qmp_usb_legacy_probe(struct platform_device *pdev)
ret = phy_pipe_clk_register(qmp, np);
if (ret)
- goto err_node_put;
+ goto err_pm_put;
qmp->phy = devm_phy_create(dev, np, &qmp_usb_legacy_phy_ops);
if (IS_ERR(qmp->phy)) {
ret = PTR_ERR(qmp->phy);
dev_err(dev, "failed to create PHY: %d\n", ret);
- goto err_node_put;
+ goto err_pm_put;
}
phy_set_drvdata(qmp->phy, qmp);
+ pm_runtime_put(dev);
+
of_node_put(np);
phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
return PTR_ERR_OR_ZERO(phy_provider);
+err_pm_put:
+ pm_runtime_put_noidle(dev);
err_node_put:
of_node_put(np);
return ret;
--
2.34.1
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v4 3/4] phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend
2026-07-20 14:53 ` [PATCH v4 3/4] phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend Loic Poulain
@ 2026-07-20 15:10 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-07-20 15:10 UTC (permalink / raw)
To: Loic Poulain; +Cc: neil.armstrong, olteanv, vkoul, linux-phy
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Asynchronous `pm_runtime_put()` combined with inverted devres cleanup order introduces a Use-After-Free of `qmp->phy` on probe error.
Pre-existing issues:
- [Low] The `dev_pm_ops` structure is assigned unconditionally to the driver's `.pm` field without the `pm_ptr()` wrapper, producing dead code when PM is disabled.
--
--- Patch [3]: [PATCH v4 3/4] phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend ---
commit 3b25be245d58901212b78dd8afabc38a9e9e8abe
Author: Loic Poulain <loic.poulain@oss.qualcomm.com>
phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend
This commit addresses a possible NULL pointer dereference in the runtime
suspend path. It takes a runtime PM usage reference with
pm_runtime_get_noresume() before enabling PM and releases it only after
the PHY is created, preventing the suspend callback from executing while
the pointer is uninitialized.
> diff --git a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
> index 8bf951b0490cf..8b91eec0b5eee 100644
> --- a/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
> +++ b/drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c
> @@ -1277,10 +1277,16 @@ static int qmp_usb_legacy_probe(struct platform_device *pdev)
> if (ret)
> goto err_node_put;
>
> + /*
> + * Enable runtime PM before creating the PHY, phy_create() only enables
> + * it on the PHY device if already enabled on the parent. Hold a usage
> + * reference so callbacks cannot run before qmp->phy is assigned.
> + */
> + pm_runtime_get_noresume(dev);
> pm_runtime_set_active(dev);
> ret = devm_pm_runtime_enable(dev);
> if (ret)
> - goto err_node_put;
> + goto err_pm_put;
[ ... ]
> qmp->phy = devm_phy_create(dev, np, &qmp_usb_legacy_phy_ops);
> if (IS_ERR(qmp->phy)) {
> ret = PTR_ERR(qmp->phy);
> dev_err(dev, "failed to create PHY: %d\n", ret);
> - goto err_node_put;
> + goto err_pm_put;
> }
>
> phy_set_drvdata(qmp->phy, qmp);
>
> + pm_runtime_put(dev);
> +
[Severity: High]
Does this asynchronous pm_runtime_put() introduce a use-after-free on the
probe error path?
If a user enables runtime PM via sysfs (setting power/control to auto) before
or during probe, this call drops the usage counter to zero and queues an
asynchronous suspend.
If devm_of_phy_provider_register() subsequently fails, probe returns an error
and devres unwinds in reverse order:
1. devm_phy_create() unwinds, freeing qmp->phy.
2. devm_pm_runtime_enable() unwinds, calling pm_runtime_disable().
If the queued asynchronous suspend executes after qmp->phy is freed but before
pm_runtime_disable() completes, qmp_usb_legacy_runtime_suspend() will
dereference the freed pointer:
drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c:qmp_usb_legacy_runtime_suspend()
if (!qmp->phy->init_count) {
Would using pm_runtime_put_sync() here ensure any pending suspend completes
before a potential devres cleanup?
> of_node_put(np);
>
> phy_provider = devm_of_phy_provider_register(dev, of_phy_simple_xlate);
>
> return PTR_ERR_OR_ZERO(phy_provider);
[ ... ]
[Severity: Low]
This is a pre-existing issue, but I noticed the dev_pm_ops structure is
assigned unconditionally to the driver:
drivers/phy/qualcomm/phy-qcom-qmp-usb-legacy.c:qmp_usb_legacy_driver
.pm = &qmp_usb_legacy_pm_ops,
If CONFIG_PM is disabled, does this leave qmp_usb_legacy_pm_ops and its
referenced callbacks as dead code?
Could pm_ptr() be used here to allow the compiler to optimize away the unused
structure when PM support is not built in?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260720-qcom-usb-phy-fix-null-v4-0-4d2a0f06d53a@oss.qualcomm.com?part=3
--
linux-phy mailing list
linux-phy@lists.infradead.org
https://lists.infradead.org/mailman/listinfo/linux-phy
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v4 4/4] phy: qcom: snps-femto-v2: Fix possible NULL-deref on early runtime suspend
2026-07-20 14:53 [PATCH v4 0/4] phy: qcom: Fix possible NULL-deref and runtime PM race conditions Loic Poulain
` (2 preceding siblings ...)
2026-07-20 14:53 ` [PATCH v4 3/4] phy: qcom: qmp-usb-legacy: Fix possible NULL-deref on early runtime suspend Loic Poulain
@ 2026-07-20 14:53 ` Loic Poulain
3 siblings, 0 replies; 6+ messages in thread
From: Loic Poulain @ 2026-07-20 14:53 UTC (permalink / raw)
To: Vinod Koul, Neil Armstrong, Dmitry Baryshkov, Wesley Cheng
Cc: linux-arm-msm, linux-phy, linux-kernel, Dmitry Baryshkov,
Konrad Dybcio, Abel Vesa, Loic Poulain
Runtime PM must be enabled before creating the PHY, since phy_create()
only enables runtime PM on the PHY device if it is already enabled on
this parent device. However, the runtime PM callbacks dereference the
hsphy instance, which is not yet ready, leaving a window where a suspend
callback may trigger a NULL pointer dereference.
Take a runtime PM usage reference with pm_runtime_get_noresume() before
enabling runtime PM and release it once the PHY has been created, so that
no runtime suspend can run before the PHY is ready. This also prevents 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 | 17 ++++++++++++++---
1 file changed, 14 insertions(+), 3 deletions(-)
diff --git a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
index eb0b0f61d98e03963bf92e5c822334f52636abe7..b1a85518fed0a7a35516d16da3ebf69afe760e40 100644
--- a/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
+++ b/drivers/phy/qualcomm/phy-qcom-snps-femto-v2.c
@@ -599,8 +599,18 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
return dev_err_probe(dev, ret,
"failed to get regulator supplies\n");
+ /*
+ * Enable runtime PM before creating the PHY, phy_create() only enables
+ * it on the PHY device if already enabled on the parent. Hold a usage
+ * reference so callbacks cannot run before the PHY is ready.
+ */
+ pm_runtime_get_noresume(dev);
pm_runtime_set_active(dev);
- pm_runtime_enable(dev);
+ ret = devm_pm_runtime_enable(dev);
+ if (ret) {
+ pm_runtime_put_noidle(dev);
+ return ret;
+ }
/*
* Prevent runtime pm from being ON by default. Users can enable
* it using power/control in sysfs.
@@ -611,6 +621,7 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
if (IS_ERR(generic_phy)) {
ret = PTR_ERR(generic_phy);
dev_err(dev, "failed to create phy, %d\n", ret);
+ pm_runtime_put_noidle(dev);
return ret;
}
hsphy->phy = generic_phy;
@@ -622,8 +633,8 @@ static int qcom_snps_hsphy_probe(struct platform_device *pdev)
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);
+
+ pm_runtime_put(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
^ permalink raw reply related [flat|nested] 6+ messages in thread