linux-kernel - [PATCH RFC 10/10] clk: Fix the ABBA locking issue with runtime PM subcalls

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20250326-cross-lock-dep-v1-10-3199e49e8652@bootlin.com>
Date: Wed, 26 Mar 2025 19:26:25 +0100
From: Miquel Raynal <miquel.raynal@...tlin.com>
To: "Rafael J. Wysocki" <rafael@...nel.org>, Pavel Machek <pavel@....cz>, 
 Len Brown <len.brown@...el.com>, 
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
 Danilo Krummrich <dakr@...nel.org>, 
 Michael Turquette <mturquette@...libre.com>, 
 Stephen Boyd <sboyd@...nel.org>
Cc: Thomas Petazzoni <thomas.petazzoni@...tlin.com>, 
 linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org, 
 linux-clk@...r.kernel.org, Chen-Yu Tsai <wenst@...omium.org>, 
 Lucas Stach <l.stach@...gutronix.de>, 
 Laurent Pinchart <laurent.pinchart@...asonboard.com>, 
 Marek Vasut <marex@...x.de>, Ulf Hansson <ulf.hansson@...aro.org>, 
 Kevin Hilman <khilman@...nel.org>, Fabio Estevam <festevam@...x.de>, 
 Jacky Bai <ping.bai@....com>, Peng Fan <peng.fan@....com>, 
 Shawn Guo <shawnguo@...nel.org>, Shengjiu Wang <shengjiu.wang@....com>, 
 linux-imx@....com, Ian Ray <ian.ray@...ealthcare.com>, 
 Hervé Codina <herve.codina@...tlin.com>, 
 Luca Ceresoli <luca.ceresoli@...tlin.com>, 
 Saravana Kannan <saravanak@...gle.com>, 
 Miquel Raynal <miquel.raynal@...tlin.com>
Subject: [PATCH RFC 10/10] clk: Fix the ABBA locking issue with runtime PM
 subcalls

The clock subsystem is calling runtime PM callbacks after having
acquired its own lock, which is in general problematic, especially
because when PM callbacks enter the power domain subsystem, we have the
following scenario:
mutex_lock(prepare_lock)
mutex_lock(genpd_lock)
But on the other side, devices may enable power domains, which
themselves might require clocks, forcing the following path:
mutex_lock(genpd_lock)
mutex_lock(prepare_lock)

The clk core has been modified in order to avoid the need for "late"
runtime PM calls (ie. inside the clk prepare_lock), so what remains to
be done is to simply remove these inner runtime calls.

Signed-off-by: Miquel Raynal <miquel.raynal@...tlin.com>
---
 drivers/clk/clk.c | 31 +++++--------------------------
 1 file changed, 5 insertions(+), 26 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c
index 26af3a134fa7b9d7f4a77ff473df7e79fd465789..652551860201f2d4ed606c55079dc4fb655d9fa0 100644
--- a/drivers/clk/clk.c
+++ b/drivers/clk/clk.c
@@ -1961,10 +1961,9 @@ static unsigned long clk_recalc(struct clk_core *core,
 {
 	unsigned long rate = parent_rate;

-	if (core->ops->recalc_rate && !clk_pm_runtime_get(core)) {
+	if (core->ops->recalc_rate)
 		rate = core->ops->recalc_rate(core->hw, parent_rate);
-		clk_pm_runtime_put(core);
-	}
+
 	return rate;
 }

@@ -2458,9 +2457,6 @@ static void clk_change_rate(struct clk_core *core)
 		best_parent_rate = core->parent->rate;
 	}

-	if (clk_pm_runtime_get(core))
-		return;
-
 	if (core->flags & CLK_SET_RATE_UNGATE) {
 		clk_core_prepare(core);
 		clk_core_enable_lock(core);
@@ -2523,8 +2519,6 @@ static void clk_change_rate(struct clk_core *core)
 	/* handle the new child who might not be in core->children yet */
 	if (core->new_child)
 		clk_change_rate(core->new_child);
-
-	clk_pm_runtime_put(core);
 }

 static unsigned long clk_core_req_round_rate_nolock(struct clk_core *core,
@@ -2562,7 +2556,6 @@ static int clk_core_set_rate_nolock(struct clk_core *core,
 {
 	struct clk_core *top, *fail_clk;
 	unsigned long rate;
-	int ret;

 	if (!core)
 		return 0;
@@ -2582,28 +2575,21 @@ static int clk_core_set_rate_nolock(struct clk_core *core,
 	if (!top)
 		return -EINVAL;

-	ret = clk_pm_runtime_get(core);
-	if (ret)
-		return ret;
-
 	/* notify that we are about to change rates */
 	fail_clk = clk_propagate_rate_change(top, PRE_RATE_CHANGE);
 	if (fail_clk) {
 		pr_debug("%s: failed to set %s rate\n", __func__,
 				fail_clk->name);
 		clk_propagate_rate_change(top, ABORT_RATE_CHANGE);
-		ret = -EBUSY;
-		goto err;
+		return -EBUSY;
 	}

 	/* change the rates */
 	clk_change_rate(top);

 	core->req_rate = req_rate;
-err:
-	clk_pm_runtime_put(core);

-	return ret;
+	return 0;
 }

 /**
@@ -2953,16 +2939,12 @@ static int clk_core_set_parent_nolock(struct clk_core *core,
 		p_rate = parent->rate;
 	}

-	ret = clk_pm_runtime_get(core);
-	if (ret)
-		return ret;
-
 	/* propagate PRE_RATE_CHANGE notifications */
 	ret = __clk_speculate_rates(core, p_rate);

 	/* abort if a driver objects */
 	if (ret & NOTIFY_STOP_MASK)
-		goto runtime_put;
+		return ret;

 	/* do the re-parent */
 	ret = __clk_set_parent(core, parent, p_index);
@@ -2975,9 +2957,6 @@ static int clk_core_set_parent_nolock(struct clk_core *core,
 		__clk_recalc_accuracies(core);
 	}

-runtime_put:
-	clk_pm_runtime_put(core);
-
 	return ret;
 }

-- 
2.48.1