[PATCH] clk: visconti: prevent array overflow in visconti_clk_register_gates()

[PATCH] clk: visconti: prevent array overflow in visconti_clk_register_gates()

[Dan Carpenter]

This code was using -1 to represent that there was no reset function.
Unfortunately, the -1 was stored in u8 so the if (clks[i].rs_id >= 0)
condition was always true.  This lead to an out of bounds access in
visconti_clk_register_gates().

Fixes: b4cbe606dc36 ("clk: visconti: Add support common clock driver and reset driver")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
 drivers/clk/visconti/clkc.h          | 3 +++
 drivers/clk/visconti/clkc-tmpv770x.c | 2 +-
 drivers/clk/visconti/clkc.c          | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/clk/visconti/clkc.h b/drivers/clk/visconti/clkc.h
index 09ed82ff64e4..8756a1ec42ef 100644
--- a/drivers/clk/visconti/clkc.h
+++ b/drivers/clk/visconti/clkc.h
@@ -73,4 +73,7 @@ int visconti_clk_register_gates(struct visconti_clk_provider *data,
 				 int num_gate,
 				 const struct visconti_reset_data *reset,
 				 spinlock_t *lock);
+
+#define NO_RESET 0xFF
+
 #endif /* _VISCONTI_CLKC_H_ */
diff --git a/drivers/clk/visconti/clkc-tmpv770x.c b/drivers/clk/visconti/clkc-tmpv770x.c
index c2b2f41a85a4..6c753b2cb558 100644
--- a/drivers/clk/visconti/clkc-tmpv770x.c
+++ b/drivers/clk/visconti/clkc-tmpv770x.c
@@ -176,7 +176,7 @@ static const struct visconti_clk_gate_table clk_gate_tables[] = {
 	{ TMPV770X_CLK_WRCK, "wrck",
 		clks_parent_data, ARRAY_SIZE(clks_parent_data),
 		0, 0x68, 0x168, 9, 32,
-		-1, }, /* No reset */
+		NO_RESET, },
 	{ TMPV770X_CLK_PICKMON, "pickmon",
 		clks_parent_data, ARRAY_SIZE(clks_parent_data),
 		0, 0x10, 0x110, 8, 4,
diff --git a/drivers/clk/visconti/clkc.c b/drivers/clk/visconti/clkc.c
index 56a8a4ffebca..d0b193b5d0b3 100644
--- a/drivers/clk/visconti/clkc.c
+++ b/drivers/clk/visconti/clkc.c
@@ -147,7 +147,7 @@ int visconti_clk_register_gates(struct visconti_clk_provider *ctx,
 		if (!dev_name)
 			return -ENOMEM;
 
-		if (clks[i].rs_id >= 0) {
+		if (clks[i].rs_id != NO_RESET) {
 			rson_offset = reset[clks[i].rs_id].rson_offset;
 			rsoff_offset = reset[clks[i].rs_id].rsoff_offset;
 			rs_idx = reset[clks[i].rs_id].rs_idx;
-- 
2.20.1

RE: [PATCH] clk: visconti: prevent array overflow in visconti_clk_register_gates()

[nobuhiro1.iwamatsu]

Hi Dan,

Thanks for your patch.
And I forgot to update and fix on the report you sent me before, sorry.

> -----Original Message-----
> From: Dan Carpenter <dan.carpenter@oracle.com>
> Sent: Wednesday, March 16, 2022 5:36 PM
> To: Michael Turquette <mturquette@baylibre.com>; iwamatsu nobuhiro(岩松
> 信洋 □ＳＷＣ◯ＡＣＴ) <nobuhiro1.iwamatsu@toshiba.co.jp>
> Cc: Stephen Boyd <sboyd@kernel.org>; linux-clk@vger.kernel.org;
> kernel-janitors@vger.kernel.org
> Subject: [PATCH] clk: visconti: prevent array overflow in
> visconti_clk_register_gates()
> 
> This code was using -1 to represent that there was no reset function.
> Unfortunately, the -1 was stored in u8 so the if (clks[i].rs_id >= 0) condition
> was always true.  This lead to an out of bounds access in
> visconti_clk_register_gates().
> 
> Fixes: b4cbe606dc36 ("clk: visconti: Add support common clock driver and
> reset driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Looks good to me.
Acked-by: Nobuhiro Iwamatsu <nobuhiro1.iwamatsu@toshiba.co.jp>

Best regards,
  Nobuhiro

> ---
>  drivers/clk/visconti/clkc.h          | 3 +++
>  drivers/clk/visconti/clkc-tmpv770x.c | 2 +-
>  drivers/clk/visconti/clkc.c          | 2 +-
>  3 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/clk/visconti/clkc.h b/drivers/clk/visconti/clkc.h index
> 09ed82ff64e4..8756a1ec42ef 100644
> --- a/drivers/clk/visconti/clkc.h
> +++ b/drivers/clk/visconti/clkc.h
> @@ -73,4 +73,7 @@ int visconti_clk_register_gates(struct
> visconti_clk_provider *data,
>  				 int num_gate,
>  				 const struct visconti_reset_data *reset,
>  				 spinlock_t *lock);
> +
> +#define NO_RESET 0xFF
> +
>  #endif /* _VISCONTI_CLKC_H_ */
> diff --git a/drivers/clk/visconti/clkc-tmpv770x.c
> b/drivers/clk/visconti/clkc-tmpv770x.c
> index c2b2f41a85a4..6c753b2cb558 100644
> --- a/drivers/clk/visconti/clkc-tmpv770x.c
> +++ b/drivers/clk/visconti/clkc-tmpv770x.c
> @@ -176,7 +176,7 @@ static const struct visconti_clk_gate_table
> clk_gate_tables[] = {
>  	{ TMPV770X_CLK_WRCK, "wrck",
>  		clks_parent_data, ARRAY_SIZE(clks_parent_data),
>  		0, 0x68, 0x168, 9, 32,
> -		-1, }, /* No reset */
> +		NO_RESET, },
>  	{ TMPV770X_CLK_PICKMON, "pickmon",
>  		clks_parent_data, ARRAY_SIZE(clks_parent_data),
>  		0, 0x10, 0x110, 8, 4,
> diff --git a/drivers/clk/visconti/clkc.c b/drivers/clk/visconti/clkc.c index
> 56a8a4ffebca..d0b193b5d0b3 100644
> --- a/drivers/clk/visconti/clkc.c
> +++ b/drivers/clk/visconti/clkc.c
> @@ -147,7 +147,7 @@ int visconti_clk_register_gates(struct
> visconti_clk_provider *ctx,
>  		if (!dev_name)
>  			return -ENOMEM;
> 
> -		if (clks[i].rs_id >= 0) {
> +		if (clks[i].rs_id != NO_RESET) {
>  			rson_offset = reset[clks[i].rs_id].rson_offset;
>  			rsoff_offset = reset[clks[i].rs_id].rsoff_offset;
>  			rs_idx = reset[clks[i].rs_id].rs_idx;
> --
> 2.20.1

Re: [PATCH] clk: visconti: prevent array overflow in visconti_clk_register_gates()

[Stephen Boyd]

Quoting Dan Carpenter (2022-03-16 01:35:33)
> This code was using -1 to represent that there was no reset function.
> Unfortunately, the -1 was stored in u8 so the if (clks[i].rs_id >= 0)
> condition was always true.  This lead to an out of bounds access in
> visconti_clk_register_gates().
> 
> Fixes: b4cbe606dc36 ("clk: visconti: Add support common clock driver and reset driver")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---

Applied to clk-next