[PATCH 0/2] Fix NULL dereference in clk_composite_set_rate()
On sandbox it's possible to trigger NULL dereference when setting rate of a composite clock. It happens because sandbox composite divider does not implement set_rate() operation. This series adds NULL check and a test cases for clk_set_rate().
Igor Prusov (2): clk: Check that composite clock's div has set_rate() dm: test: clk: Add test for ccf clk_set_rate()
drivers/clk/clk-composite.c | 2 +- test/dm/clk_ccf.c | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-)
It's possible for composite clocks to have a divider that does not implement set_rate() operation. For example, sandbox_clk_composite() registers composite clock with a divider that only has get_rate(). Currently clk_composite_set_rate() only checks thate rate_ops are present, so for sandbox it will cause NULL dereference during clk_set_rate().
This patch adds rate_ops->set_rate check tp clk_composite_set_rate().
Signed-off-by: Igor Prusov ivprusov@salutedevices.com ---
drivers/clk/clk-composite.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c index 6eb2b8133a..d2e5a1ae40 100644 --- a/drivers/clk/clk-composite.c +++ b/drivers/clk/clk-composite.c @@ -66,7 +66,7 @@ static ulong clk_composite_set_rate(struct clk *clk, unsigned long rate) const struct clk_ops *rate_ops = composite->rate_ops; struct clk *clk_rate = composite->rate;
- if (rate && rate_ops) + if (rate && rate_ops && rate_ops->set_rate) return rate_ops->set_rate(clk_rate, rate); else return clk_get_rate(clk);
On 12/5/23 18:23, Igor Prusov wrote:
It's possible for composite clocks to have a divider that does not implement set_rate() operation. For example, sandbox_clk_composite() registers composite clock with a divider that only has get_rate(). Currently clk_composite_set_rate() only checks thate rate_ops are present, so for sandbox it will cause NULL dereference during clk_set_rate().
This patch adds rate_ops->set_rate check tp clk_composite_set_rate().
Signed-off-by: Igor Prusov ivprusov@salutedevices.com
drivers/clk/clk-composite.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/clk-composite.c b/drivers/clk/clk-composite.c index 6eb2b8133a..d2e5a1ae40 100644 --- a/drivers/clk/clk-composite.c +++ b/drivers/clk/clk-composite.c @@ -66,7 +66,7 @@ static ulong clk_composite_set_rate(struct clk *clk, unsigned long rate) const struct clk_ops *rate_ops = composite->rate_ops; struct clk *clk_rate = composite->rate;
- if (rate && rate_ops)
- if (rate && rate_ops && rate_ops->set_rate) return rate_ops->set_rate(clk_rate, rate); else return clk_get_rate(clk);
Reviewed-by: Sean Anderson seanga2@gmail.com
Add a simple test case which sets clock rate to its current value.
Signed-off-by: Igor Prusov ivprusov@salutedevices.com ---
test/dm/clk_ccf.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/test/dm/clk_ccf.c b/test/dm/clk_ccf.c index e4ebb93cda..3b23982541 100644 --- a/test/dm/clk_ccf.c +++ b/test/dm/clk_ccf.c @@ -63,6 +63,9 @@ static int dm_test_clk_ccf(struct unit_test_state *uts) rate = clk_get_parent_rate(clk); ut_asserteq(rate, 60000000);
+ rate = clk_set_rate(clk, 60000000); + ut_asserteq(rate, -ENOSYS); + rate = clk_get_rate(clk); ut_asserteq(rate, 60000000);
@@ -87,6 +90,9 @@ static int dm_test_clk_ccf(struct unit_test_state *uts) ut_asserteq_str("pll3_80m", pclk->dev->name); ut_asserteq(CLK_SET_RATE_PARENT, pclk->flags);
+ rate = clk_set_rate(clk, 80000000); + ut_asserteq(rate, -ENOSYS); + rate = clk_get_rate(clk); ut_asserteq(rate, 80000000);
@@ -108,6 +114,9 @@ static int dm_test_clk_ccf(struct unit_test_state *uts) rate = clk_get_rate(clk); ut_asserteq(rate, 60000000);
+ rate = clk_set_rate(clk, 60000000); + ut_asserteq(rate, 60000000); + #if CONFIG_IS_ENABLED(CLK_CCF) /* Test clk tree enable/disable */ ret = clk_get_by_id(SANDBOX_CLK_I2C_ROOT, &clk);
On 12/5/23 18:23, Igor Prusov wrote:
Add a simple test case which sets clock rate to its current value.
Signed-off-by: Igor Prusov ivprusov@salutedevices.com
test/dm/clk_ccf.c | 9 +++++++++ 1 file changed, 9 insertions(+)
diff --git a/test/dm/clk_ccf.c b/test/dm/clk_ccf.c index e4ebb93cda..3b23982541 100644 --- a/test/dm/clk_ccf.c +++ b/test/dm/clk_ccf.c @@ -63,6 +63,9 @@ static int dm_test_clk_ccf(struct unit_test_state *uts) rate = clk_get_parent_rate(clk); ut_asserteq(rate, 60000000);
- rate = clk_set_rate(clk, 60000000);
- ut_asserteq(rate, -ENOSYS);
- rate = clk_get_rate(clk); ut_asserteq(rate, 60000000);
@@ -87,6 +90,9 @@ static int dm_test_clk_ccf(struct unit_test_state *uts) ut_asserteq_str("pll3_80m", pclk->dev->name); ut_asserteq(CLK_SET_RATE_PARENT, pclk->flags);
- rate = clk_set_rate(clk, 80000000);
- ut_asserteq(rate, -ENOSYS);
- rate = clk_get_rate(clk); ut_asserteq(rate, 80000000);
@@ -108,6 +114,9 @@ static int dm_test_clk_ccf(struct unit_test_state *uts) rate = clk_get_rate(clk); ut_asserteq(rate, 60000000);
- rate = clk_set_rate(clk, 60000000);
- ut_asserteq(rate, 60000000);
- #if CONFIG_IS_ENABLED(CLK_CCF) /* Test clk tree enable/disable */ ret = clk_get_by_id(SANDBOX_CLK_I2C_ROOT, &clk);
Reviewed-by: Sean Anderson seanga2@gmail.com
On Wed, 6 Dec 2023 02:23:32 +0300, Igor Prusov wrote:
On sandbox it's possible to trigger NULL dereference when setting rate of a composite clock. It happens because sandbox composite divider does not implement set_rate() operation. This series adds NULL check and a test cases for clk_set_rate().
Igor Prusov (2): clk: Check that composite clock's div has set_rate() dm: test: clk: Add test for ccf clk_set_rate()
[...]
Applied, thanks!
[1/2] clk: Check that composite clock's div has set_rate() https://source.denx.de/u-boot/custodians/u-boot-clk/-/commit/54d7da773062 [2/2] dm: test: clk: Add test for ccf clk_set_rate() https://source.denx.de/u-boot/custodians/u-boot-clk/-/commit/9e0250321a0d
Best regards,
participants (2)
-
Igor Prusov -
Sean Anderson