linux-kernel - [PATCH v3 1/7] clkdev: Hold clocks_mutex while iterating clocks list

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]

Date:   Thu,  4 Apr 2019 14:53:38 -0700
From:   Stephen Boyd <sboyd@...nel.org>
To:     Michael Turquette <mturquette@...libre.com>,
        Stephen Boyd <sboyd@...nel.org>
Cc:     linux-kernel@...r.kernel.org, linux-clk@...r.kernel.org,
        Miquel Raynal <miquel.raynal@...tlin.com>,
        Jerome Brunet <jbrunet@...libre.com>,
        Russell King <linux@...linux.org.uk>,
        Jeffrey Hugo <jhugo@...eaurora.org>,
        Chen-Yu Tsai <wens@...e.org>,
        Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
Subject: [PATCH v3 1/7] clkdev: Hold clocks_mutex while iterating clocks list

We recently introduced a change to support devm clk lookups. That change
introduced a code-path that used clk_find() without holding the
'clocks_mutex'. Unfortunately, clk_find() iterates over the 'clocks'
list and so we need to prevent the list from being modified while
iterating over it by holding the mutex. Similarly, we don't need to hold
the 'clocks_mutex' besides when we're dereferencing the clk_lookup
pointer we find or while we're modifying/iterating the 'clocks' list.

Let's ensure that we have the mutex held while iterating the list and
fix the callers of clk_find() to only hold the mutex as long as they
need to. This should fix this race and also nicely move clk creation
outside of the 'clocks_mutex'.

Fixes: 3eee6c7d119c ("clkdev: add managed clkdev lookup registration")
Cc: Miquel Raynal <miquel.raynal@...tlin.com>
Cc: Jerome Brunet <jbrunet@...libre.com>
Cc: Russell King <linux@...linux.org.uk>
Cc: Michael Turquette <mturquette@...libre.com>
Cc: Jeffrey Hugo <jhugo@...eaurora.org>
Cc: Chen-Yu Tsai <wens@...e.org>
Cc: Matti Vaittinen <matti.vaittinen@...rohmeurope.com>
Signed-off-by: Stephen Boyd <sboyd@...nel.org>
---
 drivers/clk/clkdev.c | 38 ++++++++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/clk/clkdev.c b/drivers/clk/clkdev.c
index 8c4435c53f09..db82eee8e209 100644
--- a/drivers/clk/clkdev.c
+++ b/drivers/clk/clkdev.c
@@ -36,7 +36,7 @@ static DEFINE_MUTEX(clocks_mutex);
  * Then we take the most specific entry - with the following
  * order of precedence: dev+con > dev only > con only.
  */
-static struct clk_lookup *clk_find(const char *dev_id, const char *con_id)
+static struct clk_lookup *__clk_find(const char *dev_id, const char *con_id)
 {
 	struct clk_lookup *p, *cl = NULL;
 	int match, best_found = 0, best_possible = 0;
@@ -46,6 +46,8 @@ static struct clk_lookup *clk_find(const char *dev_id, const char *con_id)
 	if (con_id)
 		best_possible += 1;
 
+	lockdep_assert_held(&clocks_mutex);
+
 	list_for_each_entry(p, &clocks, node) {
 		match = 0;
 		if (p->dev_id) {
@@ -70,25 +72,37 @@ static struct clk_lookup *clk_find(const char *dev_id, const char *con_id)
 	return cl;
 }
 
-static struct clk *__clk_get_sys(struct device *dev, const char *dev_id,
-				 const char *con_id)
+static struct clk_hw *clk_find_hw(const char *dev_id, const char *con_id)
 {
 	struct clk_lookup *cl;
-	struct clk *clk = NULL;
+	struct clk_hw *hw = ERR_PTR(-ENOENT);
 
 	mutex_lock(&clocks_mutex);
+	cl = __clk_find(dev_id, con_id);
+	if (cl)
+		hw = cl->clk_hw;
+	mutex_unlock(&clocks_mutex);
 
-	cl = clk_find(dev_id, con_id);
-	if (!cl)
-		goto out;
+	return hw;
+}
 
-	clk = clk_hw_create_clk(dev, cl->clk_hw, dev_id, con_id);
-	if (IS_ERR(clk))
-		cl = NULL;
-out:
+static struct clk_lookup *clk_find(const char *dev_id, const char *con_id)
+{
+	struct clk_lookup *cl;
+
+	mutex_lock(&clocks_mutex);
+	cl = __clk_find(dev_id, con_id);
 	mutex_unlock(&clocks_mutex);
 
-	return cl ? clk : ERR_PTR(-ENOENT);
+	return cl;
+}
+
+static struct clk *__clk_get_sys(struct device *dev, const char *dev_id,
+				 const char *con_id)
+{
+	struct clk_hw *hw = clk_find_hw(dev_id, con_id);
+
+	return clk_hw_create_clk(dev, hw, dev_id, con_id);
 }
 
 struct clk *clk_get_sys(const char *dev_id, const char *con_id)
-- 
Sent by a computer through tubes