All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Fix NULL dereference in clk_composite_set_rate()
@ 2023-12-05 23:23 Igor Prusov
  2023-12-05 23:23 ` [PATCH 1/2] clk: Check that composite clock's div has set_rate() Igor Prusov
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Igor Prusov @ 2023-12-05 23:23 UTC (permalink / raw)
  To: u-boot
  Cc: kernel, prusovigor, Igor Prusov, Lukasz Majewski, Sean Anderson,
	Simon Glass

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(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 6+ messages in thread
* [PATCH 1/2] clk: Check that composite clock's div has set_rate()
  2023-12-05 23:23 [PATCH 0/2] Fix NULL dereference in clk_composite_set_rate() Igor Prusov
@ 2023-12-05 23:23 ` Igor Prusov
  2023-12-15 17:03   ` Sean Anderson
  2023-12-05 23:23 ` [PATCH 2/2] dm: test: clk: Add test for ccf clk_set_rate() Igor Prusov
  2023-12-15 18:37 ` [PATCH 0/2] Fix NULL dereference in clk_composite_set_rate() Sean Anderson
  2 siblings, 1 reply; 6+ messages in thread
From: Igor Prusov @ 2023-12-05 23:23 UTC (permalink / raw)
  To: u-boot; +Cc: kernel, prusovigor, Igor Prusov, Lukasz Majewski, Sean Anderson

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);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [PATCH 2/2] dm: test: clk: Add test for ccf clk_set_rate()
  2023-12-05 23:23 [PATCH 0/2] Fix NULL dereference in clk_composite_set_rate() Igor Prusov
  2023-12-05 23:23 ` [PATCH 1/2] clk: Check that composite clock's div has set_rate() Igor Prusov
@ 2023-12-05 23:23 ` Igor Prusov
  2023-12-15 17:03   ` Sean Anderson
  2023-12-15 18:37 ` [PATCH 0/2] Fix NULL dereference in clk_composite_set_rate() Sean Anderson
  2 siblings, 1 reply; 6+ messages in thread
From: Igor Prusov @ 2023-12-05 23:23 UTC (permalink / raw)
  To: u-boot; +Cc: kernel, prusovigor, Igor Prusov, Simon Glass

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);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [PATCH 1/2] clk: Check that composite clock's div has set_rate()
  2023-12-05 23:23 ` [PATCH 1/2] clk: Check that composite clock's div has set_rate() Igor Prusov
@ 2023-12-15 17:03   ` Sean Anderson
  0 siblings, 0 replies; 6+ messages in thread
From: Sean Anderson @ 2023-12-15 17:03 UTC (permalink / raw)
  To: Igor Prusov, u-boot; +Cc: kernel, prusovigor, Lukasz Majewski

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>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 2/2] dm: test: clk: Add test for ccf clk_set_rate()
  2023-12-05 23:23 ` [PATCH 2/2] dm: test: clk: Add test for ccf clk_set_rate() Igor Prusov
@ 2023-12-15 17:03   ` Sean Anderson
  0 siblings, 0 replies; 6+ messages in thread
From: Sean Anderson @ 2023-12-15 17:03 UTC (permalink / raw)
  To: Igor Prusov, u-boot; +Cc: kernel, prusovigor, Simon Glass

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>

^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [PATCH 0/2] Fix NULL dereference in clk_composite_set_rate()
  2023-12-05 23:23 [PATCH 0/2] Fix NULL dereference in clk_composite_set_rate() Igor Prusov
  2023-12-05 23:23 ` [PATCH 1/2] clk: Check that composite clock's div has set_rate() Igor Prusov
  2023-12-05 23:23 ` [PATCH 2/2] dm: test: clk: Add test for ccf clk_set_rate() Igor Prusov
@ 2023-12-15 18:37 ` Sean Anderson
  2 siblings, 0 replies; 6+ messages in thread
From: Sean Anderson @ 2023-12-15 18:37 UTC (permalink / raw)
  To: ivprusov, u-boot; +Cc: Sean Anderson, prusovigor, sjg, kernel, Lukasz Majewski

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,
-- 
Sean Anderson <seanga2@gmail.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-12-15 18:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-12-05 23:23 [PATCH 0/2] Fix NULL dereference in clk_composite_set_rate() Igor Prusov
2023-12-05 23:23 ` [PATCH 1/2] clk: Check that composite clock's div has set_rate() Igor Prusov
2023-12-15 17:03   ` Sean Anderson
2023-12-05 23:23 ` [PATCH 2/2] dm: test: clk: Add test for ccf clk_set_rate() Igor Prusov
2023-12-15 17:03   ` Sean Anderson
2023-12-15 18:37 ` [PATCH 0/2] Fix NULL dereference in clk_composite_set_rate() Sean Anderson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.