From 1c0a46e2918a1ddf42f51449e45077513dd52417 Mon Sep 17 00:00:00 2001 From: Andrew Goodbody Date: Fri, 21 Nov 2025 17:34:31 +0000 Subject: [PATCH 1/3] clk: Prevent SIGSEGV on debug If LOG_DEBUG is defined and a NULL clk is passed to clk_enable or clk_disable then an attempt is made to dereference NULL in the debug statement. Guard against this. Signed-off-by: Andrew Goodbody Reviewed-by: Patrick Delaunay --- drivers/clk/clk-uclass.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index 3dbe1ce9441..1f55dbe385b 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -660,7 +660,7 @@ int clk_enable(struct clk *clk) struct clk *clkp = NULL; int ret; - debug("%s(clk=%p name=%s)\n", __func__, clk, clk->dev->name); + debug("%s(clk=%p name=%s)\n", __func__, clk, clk ? clk->dev->name : "NULL"); if (!clk_valid(clk)) return 0; ops = clk_dev_ops(clk->dev); @@ -721,7 +721,7 @@ int clk_disable(struct clk *clk) struct clk *clkp = NULL; int ret; - debug("%s(clk=%p name=%s)\n", __func__, clk, clk->dev->name); + debug("%s(clk=%p name=%s)\n", __func__, clk, clk ? clk->dev->name : "NULL"); if (!clk_valid(clk)) return 0; ops = clk_dev_ops(clk->dev); From fe780310cfa8bf5a093894b5cd7fe85c6b02fd91 Mon Sep 17 00:00:00 2001 From: Andrew Goodbody Date: Fri, 21 Nov 2025 17:34:32 +0000 Subject: [PATCH 2/3] clk: Return value calculated by ERR_PTR In clk_set_default_get_by_id ret is passed to ERR_PTR but nothing is done with the value that this calculates which is obviously not the intention of the code. This is confirmed by the code around where this function is called. Instead return the value from ERR_PTR. Then fixup the sandbox code so that the test dm_test_clk does not fail as it relied on the broken behaviour. Finally disable part of the test that does not work correctly with CLK_AUTO_ID This issue found by Smatch. Signed-off-by: Andrew Goodbody --- arch/sandbox/include/asm/clk.h | 1 + drivers/clk/clk-uclass.c | 4 ++-- drivers/clk/clk_sandbox.c | 17 +++++++++++++++++ test/dm/clk.c | 5 +++-- 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/arch/sandbox/include/asm/clk.h b/arch/sandbox/include/asm/clk.h index 37fe49c7fcf..2d3318cdcc4 100644 --- a/arch/sandbox/include/asm/clk.h +++ b/arch/sandbox/include/asm/clk.h @@ -50,6 +50,7 @@ enum sandbox_clk_test_id { struct sandbox_clk_priv { bool probed; + struct clk clk; ulong rate[SANDBOX_CLK_ID_COUNT]; bool enabled[SANDBOX_CLK_ID_COUNT]; bool requested[SANDBOX_CLK_ID_COUNT]; diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index 1f55dbe385b..a72a67d75a1 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -199,7 +199,7 @@ static struct clk *clk_set_default_get_by_id(struct clk *clk) if (ret) { debug("%s(): could not get parent clock pointer, id %lu\n", __func__, clk->id); - ERR_PTR(ret); + return ERR_PTR(ret); } } @@ -354,7 +354,7 @@ static int clk_set_default_rates(struct udevice *dev, c = clk_set_default_get_by_id(&clk); if (IS_ERR(c)) - return PTR_ERR(c); + continue; ret = clk_set_rate(c, rates[index]); diff --git a/drivers/clk/clk_sandbox.c b/drivers/clk/clk_sandbox.c index c8c5a88c52d..cd92a6a29f5 100644 --- a/drivers/clk/clk_sandbox.c +++ b/drivers/clk/clk_sandbox.c @@ -8,8 +8,23 @@ #include #include #include +#include #include +static int sandbox_clk_of_to_plat(struct udevice *dev) +{ + struct clk *clk; + struct sandbox_clk_priv *priv = dev_get_priv(dev); + + clk = &priv->clk; + + /* FIXME: This is not allowed */ + dev_set_uclass_priv(dev, clk); + + clk->dev = dev; + return 0; +} + static ulong sandbox_clk_get_rate(struct clk *clk) { struct sandbox_clk_priv *priv = dev_get_priv(clk->dev); @@ -102,6 +117,7 @@ static int sandbox_clk_request(struct clk *clk) if (id >= SANDBOX_CLK_ID_COUNT) return -EINVAL; + priv->clk.id = id; priv->requested[id] = true; return 0; } @@ -132,6 +148,7 @@ U_BOOT_DRIVER(sandbox_clk) = { .name = "sandbox_clk", .id = UCLASS_CLK, .of_match = sandbox_clk_ids, + .of_to_plat = sandbox_clk_of_to_plat, .ops = &sandbox_clk_ops, .probe = sandbox_clk_probe, .priv_auto = sizeof(struct sandbox_clk_priv), diff --git a/test/dm/clk.c b/test/dm/clk.c index 790968e6477..c3363796498 100644 --- a/test/dm/clk.c +++ b/test/dm/clk.c @@ -89,8 +89,9 @@ static int dm_test_clk(struct unit_test_state *uts) SANDBOX_CLK_TEST_ID_SPI)); ut_asserteq(0, sandbox_clk_test_get_rate(dev_test, SANDBOX_CLK_TEST_ID_I2C)); - ut_asserteq(321, sandbox_clk_test_get_rate(dev_test, - SANDBOX_CLK_TEST_ID_DEVM1)); + if (!CONFIG_IS_ENABLED(CLK_AUTO_ID)) + ut_asserteq(321, sandbox_clk_test_get_rate(dev_test, + SANDBOX_CLK_TEST_ID_DEVM1)); ut_asserteq(0, sandbox_clk_test_get_rate(dev_test, SANDBOX_CLK_TEST_ID_DEVM2)); From 33285945518731ed459680ffb01891c4804fb6c9 Mon Sep 17 00:00:00 2001 From: Andrew Goodbody Date: Fri, 21 Nov 2025 17:34:33 +0000 Subject: [PATCH 3/3] clk: Prevent memory leak on error In clk_set_default_rates() memory is allocated to store the clock rates that are read. Direct returns fail to free this memory leading to a memory leak so instead use 'goto fail;' which will then perform the free before exiting the function. Signed-off-by: Andrew Goodbody --- drivers/clk/clk-uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/clk/clk-uclass.c b/drivers/clk/clk-uclass.c index a72a67d75a1..bf9f1041935 100644 --- a/drivers/clk/clk-uclass.c +++ b/drivers/clk/clk-uclass.c @@ -338,7 +338,7 @@ static int clk_set_default_rates(struct udevice *dev, continue; } - return ret; + goto fail; } /* This is clk provider device trying to program itself