Since commit 02fb4f008433 ("clk: clk-loongson2: Fix potential buffer overflow in flexible-array member access"), the clk provider register is failed.
The count of `clks_num` is shown below:
for (p = data; p->name; p++) clks_num++;
In fact, `clks_num` represents the number of SoC clocks and should be expressed as the maximum value of the clock binding id in use (p->id + 1).
Now we fix it to avoid the following error when trying to register a clk provider:
[ 13.409595] of_clk_hw_onecell_get: invalid index 17
Cc: stable@vger.kernel.org Cc: Gustavo A. R. Silva gustavoars@kernel.org Fixes: 02fb4f008433 ("clk: clk-loongson2: Fix potential buffer overflow in flexible-array member access") Signed-off-by: Binbin Zhou zhoubinbin@loongson.cn --- V2: - Add Gustavo A. R. Silva to cc list; - Populate the onecell data with -ENOENT error pointers to avoid returning NULL, for it is a valid clock.
Link to V1: https://lore.kernel.org/all/20241225060600.3094154-1-zhoubinbin@loongson.cn/
drivers/clk/clk-loongson2.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/clk/clk-loongson2.c b/drivers/clk/clk-loongson2.c index 6bf51d5a49a1..9c240a2308f5 100644 --- a/drivers/clk/clk-loongson2.c +++ b/drivers/clk/clk-loongson2.c @@ -294,7 +294,7 @@ static int loongson2_clk_probe(struct platform_device *pdev) return -EINVAL;
for (p = data; p->name; p++) - clks_num++; + clks_num = max(clks_num, p->id + 1);
clp = devm_kzalloc(dev, struct_size(clp, clk_data.hws, clks_num), GFP_KERNEL); @@ -309,6 +309,9 @@ static int loongson2_clk_probe(struct platform_device *pdev) clp->clk_data.num = clks_num; clp->dev = dev;
+ /* Avoid returning NULL for unused id */ + memset_p((void **)&clp->clk_data.hws, ERR_PTR(-ENOENT), clks_num); + for (i = 0; i < clks_num; i++) { p = &data[i]; switch (p->type) {
Quoting Binbin Zhou (2025-01-09 03:30:04)
diff --git a/drivers/clk/clk-loongson2.c b/drivers/clk/clk-loongson2.c index 6bf51d5a49a1..9c240a2308f5 100644 --- a/drivers/clk/clk-loongson2.c +++ b/drivers/clk/clk-loongson2.c @@ -294,7 +294,7 @@ static int loongson2_clk_probe(struct platform_device *pdev) return -EINVAL; for (p = data; p->name; p++)
clks_num++;
clks_num = max(clks_num, p->id + 1);
clp = devm_kzalloc(dev, struct_size(clp, clk_data.hws, clks_num), GFP_KERNEL); @@ -309,6 +309,9 @@ static int loongson2_clk_probe(struct platform_device *pdev) clp->clk_data.num = clks_num; clp->dev = dev;
/* Avoid returning NULL for unused id */
memset_p((void **)&clp->clk_data.hws, ERR_PTR(-ENOENT), clks_num);
This looks wrong. It's already an array of pointers, i.e. the type is 'struct clk_hw *[]' or 'struct clk_hw **' so we shouldn't need to take the address of it. Should it be
memset_p((void **)clkp->clk_data.hws, ERR_PTR(-ENOENT), clks_num);
? It's unfortunate that we have to cast here, but I guess this is the best way we can indicate that the type should be an array of pointers.
On Tue, Jan 14, 2025 at 2:50 AM Stephen Boyd sboyd@kernel.org wrote:
Quoting Binbin Zhou (2025-01-09 03:30:04)
diff --git a/drivers/clk/clk-loongson2.c b/drivers/clk/clk-loongson2.c index 6bf51d5a49a1..9c240a2308f5 100644 --- a/drivers/clk/clk-loongson2.c +++ b/drivers/clk/clk-loongson2.c @@ -294,7 +294,7 @@ static int loongson2_clk_probe(struct platform_device *pdev) return -EINVAL;
for (p = data; p->name; p++)
clks_num++;
clks_num = max(clks_num, p->id + 1); clp = devm_kzalloc(dev, struct_size(clp, clk_data.hws, clks_num), GFP_KERNEL);
@@ -309,6 +309,9 @@ static int loongson2_clk_probe(struct platform_device *pdev) clp->clk_data.num = clks_num; clp->dev = dev;
/* Avoid returning NULL for unused id */
memset_p((void **)&clp->clk_data.hws, ERR_PTR(-ENOENT), clks_num);
This looks wrong. It's already an array of pointers, i.e. the type is 'struct clk_hw *[]' or 'struct clk_hw **' so we shouldn't need to take the address of it. Should it be
memset_p((void **)clkp->clk_data.hws, ERR_PTR(-ENOENT), clks_num);
I'm very sorry, it was a cheap clerical error and I'll fix it right away.
? It's unfortunate that we have to cast here, but I guess this is the best way we can indicate that the type should be an array of pointers.
linux-stable-mirror@lists.linaro.org