diff mbox series

clk: clk-loongson2: Fix potential buffer overflow in flexible-array member access

Message ID ZzaN5MpmMr0hwHw9@kspp (mailing list archive)
State Mainlined
Commit 02fb4f0084331ef72c28d0c70fcb15d1bea369ec
Headers show
Series clk: clk-loongson2: Fix potential buffer overflow in flexible-array member access | expand

Commit Message

Gustavo A. R. Silva Nov. 14, 2024, 11:55 p.m. UTC
Flexible-array member `hws` in `struct clk_hw_onecell_data` is annotated
with the `counted_by()` attribute. This means that when memory is
allocated for this array, the _counter_, which in this case is member
`num` in the flexible structure, should be set to the maximum number of
elements the flexible array can contain, or fewer.

In this case, the total number of elements for the flexible array is
determined by variable `clks_num` when allocating heap space via
`devm_kzalloc()`, as shown below:

289         struct loongson2_clk_provider *clp;
	...
296         for (p = data; p->name; p++)
297                 clks_num++;
298
299         clp = devm_kzalloc(dev, struct_size(clp, clk_data.hws, clks_num),
300                            GFP_KERNEL);

So, `clp->clk_data.num` should be set to `clks_num` or less, and not
exceed `clks_num`, as is currently the case. Otherwise, if data is
written into `clp->clk_data.hws[clks_num]`, the instrumentation
provided by the compiler won't detect the overflow, leading to a
memory corruption bug at runtime.

Fix this issue by setting `clp->clk_data.num` to `clks_num`.

Fixes: 9796ec0bd04b ("clk: clk-loongson2: Refactor driver for adding new platforms")
Cc: stable@vger.kernel.org
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
---
 drivers/clk/clk-loongson2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Stephen Boyd Nov. 15, 2024, 1:35 a.m. UTC | #1
Quoting Gustavo A. R. Silva (2024-11-14 15:55:16)
> Flexible-array member `hws` in `struct clk_hw_onecell_data` is annotated
> with the `counted_by()` attribute. This means that when memory is
> allocated for this array, the _counter_, which in this case is member
> `num` in the flexible structure, should be set to the maximum number of
> elements the flexible array can contain, or fewer.
> 
> In this case, the total number of elements for the flexible array is
> determined by variable `clks_num` when allocating heap space via
> `devm_kzalloc()`, as shown below:
> 
> 289         struct loongson2_clk_provider *clp;
>         ...
> 296         for (p = data; p->name; p++)
> 297                 clks_num++;
> 298
> 299         clp = devm_kzalloc(dev, struct_size(clp, clk_data.hws, clks_num),
> 300                            GFP_KERNEL);
> 
> So, `clp->clk_data.num` should be set to `clks_num` or less, and not
> exceed `clks_num`, as is currently the case. Otherwise, if data is
> written into `clp->clk_data.hws[clks_num]`, the instrumentation
> provided by the compiler won't detect the overflow, leading to a
> memory corruption bug at runtime.
> 
> Fix this issue by setting `clp->clk_data.num` to `clks_num`.
> 
> Fixes: 9796ec0bd04b ("clk: clk-loongson2: Refactor driver for adding new platforms")
> Cc: stable@vger.kernel.org
> Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
> ---

Applied to clk-next
diff mbox series

Patch

diff --git a/drivers/clk/clk-loongson2.c b/drivers/clk/clk-loongson2.c
index e99ba79feec6..7082b4309c6f 100644
--- a/drivers/clk/clk-loongson2.c
+++ b/drivers/clk/clk-loongson2.c
@@ -306,7 +306,7 @@  static int loongson2_clk_probe(struct platform_device *pdev)
 		return PTR_ERR(clp->base);
 
 	spin_lock_init(&clp->clk_lock);
-	clp->clk_data.num = clks_num + 1;
+	clp->clk_data.num = clks_num;
 	clp->dev = dev;
 
 	for (i = 0; i < clks_num; i++) {