From 7045465500e465b09f09d6e5bdc260a9f1aab97b Mon Sep 17 00:00:00 2001

From: Lukasz Bartosik <lb@semihalf.com>

Date: Fri, 2 Apr 2021 00:51:49 +0200

Subject: [PATCH] clk: fix invalid usage of list cursor in unregister

Git-commit: 7045465500e465b09f09d6e5bdc260a9f1aab97b

Patch-mainline: v5.12-rc7

References: git-fixes

Fix invalid usage of a list_for_each_entry cursor in

clk_notifier_unregister(). When list is empty or if the list

is completely traversed (without breaking from the loop on one

of the entries) then the list cursor does not point to a valid

entry and therefore should not be used. The patch fixes a logical

bug that hasn't been seen in pratice however it is analogus

to the bug fixed in clk_notifier_register().

The issue was dicovered when running 5.12-rc1 kernel on x86_64

with KASAN enabled:

Bug: KASAN: global-out-of-bounds in clk_notifier_register+0xab/0x230

Read of size 8 at addr ffffffffa0d10588 by task swapper/0/1

Cpu: 1 PID: 1 Comm: swapper/0 Not tainted 5.12.0-rc1 #1

Hardware name: Google Caroline/Caroline,

BIOS Google_Caroline.7820.430.0 07/20/2018

Call Trace:

 dump_stack+0xee/0x15c

 print_address_description+0x1e/0x2dc

 kasan_report+0x188/0x1ce

 ? clk_notifier_register+0xab/0x230

 ? clk_prepare_lock+0x15/0x7b

 ? clk_notifier_register+0xab/0x230

 clk_notifier_register+0xab/0x230

 dw8250_probe+0xc01/0x10d4

 ...

 Memory state around the buggy address:

  ffffffffa0d10480: 00 00 00 00 00 03 f9 f9 f9 f9 f9 f9 00 00 00 00

  ffffffffa0d10500: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 f9 f9

 >ffffffffa0d10580: f9 f9 f9 f9 00 00 00 00 00 00 00 00 00 00 00 00

                          ^

  ffffffffa0d10600: 00 00 00 00 00 00 f9 f9 f9 f9 f9 f9 00 00 00 00

  ffffffffa0d10680: 00 00 00 00 00 00 00 00 f9 f9 f9 f9 00 00 00 00

  ==================================================================

Fixes: b2476490ef11 ("clk: introduce the common clock framework")

Reported-by: Lukasz Majczak <lma@semihalf.com>

Signed-off-by: Lukasz Bartosik <lb@semihalf.com>

Link: https://lore.kernel.org/r/20210401225149.18826-2-lb@semihalf.com

Signed-off-by: Stephen Boyd <sboyd@kernel.org>

Acked-by: Takashi Iwai <tiwai@suse.de>

---

 drivers/clk/clk.c | 30 +++++++++++++-----------------

 1 file changed, 13 insertions(+), 17 deletions(-)

diff --git a/drivers/clk/clk.c b/drivers/clk/clk.c

index 16634d5912be..39cfc6c6a8d2 100644

--- a/drivers/clk/clk.c

+++ b/drivers/clk/clk.c

@@ -4394,32 +4394,28 @@ EXPORT_SYMBOL_GPL(clk_notifier_register);

  */

 int clk_notifier_unregister(struct clk *clk, struct notifier_block *nb)

 {

-	struct clk_notifier *cn = NULL;

-	int ret = -EINVAL;

+	struct clk_notifier *cn;

+	int ret = -ENOENT;

 	if (!clk || !nb)

 		return -EINVAL;

 	clk_prepare_lock();

-	list_for_each_entry(cn, &clk_notifier_list, node)

-		if (cn->clk == clk)

-			break;

-

-	if (cn->clk == clk) {

-		ret = srcu_notifier_chain_unregister(&cn->notifier_head, nb);

+	list_for_each_entry(cn, &clk_notifier_list, node) {

+		if (cn->clk == clk) {

+			ret = srcu_notifier_chain_unregister(&cn->notifier_head, nb);

-		clk->core->notifier_count--;

+			clk->core->notifier_count--;

-		/* XXX the notifier code should handle this better */

-		if (!cn->notifier_head.head) {

-			srcu_cleanup_notifier_head(&cn->notifier_head);

-			list_del(&cn->node);

-			kfree(cn);

+			/* XXX the notifier code should handle this better */

+			if (!cn->notifier_head.head) {

+				srcu_cleanup_notifier_head(&cn->notifier_head);

+				list_del(&cn->node);

+				kfree(cn);

+			}

+			break;

 		}

-

-	} else {

-		ret = -ENOENT;

 	}

 	clk_prepare_unlock();

-- 

2.26.2