[PATCH] hwrng: omap - balance runtime PM and clocks on probe-defer paths

William Theesfeld posted 1 patch 2 days, 10 hours ago
drivers/char/hw_random/omap-rng.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
[PATCH] hwrng: omap - balance runtime PM and clocks on probe-defer paths
Posted by William Theesfeld 2 days, 10 hours ago
omap_rng_probe() calls pm_runtime_enable() and pm_runtime_resume_and_get()
to bring the device up.  If either devm_clk_get() call subsequently
returns -EPROBE_DEFER, the function returns -EPROBE_DEFER directly,
leaking the runtime PM usage counter taken by resume_and_get() and
leaving pm_runtime enabled.

Convert both early returns to set ret and jump to err_register, which
already performs the matching pm_runtime_put_sync() + pm_runtime_disable()
unwind.  Because devm_clk_get() returns ERR_PTR on failure (not NULL)
and err_register calls clk_disable_unprepare() unconditionally, also
NULL out the failed clk pointers before the goto so that
clk_disable_unprepare() (which only handles NULL safely, not ERR_PTR)
does not deref an error pointer.

While here, NULL out priv->clk and priv->clk_reg in the existing
"optional clock not present" else branches.  In that pre-existing case
the pointer was left as ERR_PTR, and the unconditional
clk_disable_unprepare() in omap_rng_remove() would have dereferenced
it on driver unbind.  No functional change for systems where both
clocks are present.

Found by smatch ("missing unwind goto?").

Signed-off-by: William Theesfeld <william@theesfeld.net>
---
 drivers/char/hw_random/omap-rng.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/drivers/char/hw_random/omap-rng.c b/drivers/char/hw_random/omap-rng.c
index 5e8b50f15..1902865a9 100644
--- a/drivers/char/hw_random/omap-rng.c
+++ b/drivers/char/hw_random/omap-rng.c
@@ -459,8 +459,11 @@ static int omap_rng_probe(struct platform_device *pdev)
 	}
 
 	priv->clk = devm_clk_get(&pdev->dev, NULL);
-	if (PTR_ERR(priv->clk) == -EPROBE_DEFER)
-		return -EPROBE_DEFER;
+	if (PTR_ERR(priv->clk) == -EPROBE_DEFER) {
+		priv->clk = NULL;
+		ret = -EPROBE_DEFER;
+		goto err_register;
+	}
 	if (!IS_ERR(priv->clk)) {
 		ret = clk_prepare_enable(priv->clk);
 		if (ret) {
@@ -468,11 +471,21 @@ static int omap_rng_probe(struct platform_device *pdev)
 				"Unable to enable the clk: %d\n", ret);
 			goto err_register;
 		}
+	} else {
+		/*
+		 * No optional clock present; make priv->clk safe for the
+		 * unconditional clk_disable_unprepare() in err_register and
+		 * in omap_rng_remove().
+		 */
+		priv->clk = NULL;
 	}
 
 	priv->clk_reg = devm_clk_get(&pdev->dev, "reg");
-	if (PTR_ERR(priv->clk_reg) == -EPROBE_DEFER)
-		return -EPROBE_DEFER;
+	if (PTR_ERR(priv->clk_reg) == -EPROBE_DEFER) {
+		priv->clk_reg = NULL;
+		ret = -EPROBE_DEFER;
+		goto err_register;
+	}
 	if (!IS_ERR(priv->clk_reg)) {
 		ret = clk_prepare_enable(priv->clk_reg);
 		if (ret) {
@@ -481,6 +494,9 @@ static int omap_rng_probe(struct platform_device *pdev)
 				ret);
 			goto err_register;
 		}
+	} else {
+		/* Same rationale as for priv->clk above. */
+		priv->clk_reg = NULL;
 	}
 
 	ret = (dev->of_node) ? of_get_omap_rng_device_details(priv, pdev) :
-- 
2.54.0