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
Fixes: 02fb4f008433 ("clk: clk-loongson2: Fix potential buffer overflow in flexible-array member access") Cc: stable@vger.kernel.org Signed-off-by: Binbin Zhou zhoubinbin@loongson.cn --- drivers/clk/clk-loongson2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/clk-loongson2.c b/drivers/clk/clk-loongson2.c index 6bf51d5a49a1..b1b2038acd0b 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);
Quoting Binbin Zhou (2024-12-24 22:05:59)
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
Fixes: 02fb4f008433 ("clk: clk-loongson2: Fix potential buffer overflow in flexible-array member access") Cc: stable@vger.kernel.org Signed-off-by: Binbin Zhou zhoubinbin@loongson.cn
It's common practice to Cc the author of a patch in Fixes. Please do it next time.
drivers/clk/clk-loongson2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/clk-loongson2.c b/drivers/clk/clk-loongson2.c index 6bf51d5a49a1..b1b2038acd0b 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);
NULL is a valid clk. Either fill the onecell data with -ENOENT error pointers, or stop using it and implement a custom version of of_clk_hw_onecell_get() that doesn't allow invalid clks to be requested from this provider.
clp = devm_kzalloc(dev, struct_size(clp, clk_data.hws, clks_num), GFP_KERNEL);
Hi Stephen:
Thanks for your review.
On Wed, Jan 8, 2025 at 5:25 AM Stephen Boyd sboyd@kernel.org wrote:
Quoting Binbin Zhou (2024-12-24 22:05:59)
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
Fixes: 02fb4f008433 ("clk: clk-loongson2: Fix potential buffer overflow in flexible-array member access") Cc: stable@vger.kernel.org Signed-off-by: Binbin Zhou zhoubinbin@loongson.cn
It's common practice to Cc the author of a patch in Fixes. Please do it next time.
Oh, sorry it's my fault, I will do it next time.
drivers/clk/clk-loongson2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/clk/clk-loongson2.c b/drivers/clk/clk-loongson2.c index 6bf51d5a49a1..b1b2038acd0b 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);NULL is a valid clk. Either fill the onecell data with -ENOENT error pointers, or stop using it and implement a custom version of of_clk_hw_onecell_get() that doesn't allow invalid clks to be requested from this provider.
Emm... Just in case, how about setting all items to ERR_PTR(-ENOENT) before assigning them. This is shown below:
while (--clk_num >= 0) clp->clk_data.hws[clk_num] = ERR_PTR(-ENOENT);
clp = devm_kzalloc(dev, struct_size(clp, clk_data.hws, clks_num), GFP_KERNEL);
Quoting Binbin Zhou (2025-01-07 17:41:43)
On Wed, Jan 8, 2025 at 5:25 AM Stephen Boyd sboyd@kernel.org wrote:
Quoting Binbin Zhou (2024-12-24 22:05:59)
diff --git a/drivers/clk/clk-loongson2.c b/drivers/clk/clk-loongson2.c index 6bf51d5a49a1..b1b2038acd0b 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);NULL is a valid clk. Either fill the onecell data with -ENOENT error pointers, or stop using it and implement a custom version of of_clk_hw_onecell_get() that doesn't allow invalid clks to be requested from this provider.
Emm... Just in case, how about setting all items to ERR_PTR(-ENOENT) before assigning them. This is shown below:
while (--clk_num >= 0) clp->clk_data.hws[clk_num] = ERR_PTR(-ENOENT);
Or something like:
memset_p(&clk->clk_data.hws, ERR_PTR(-ENOENT), clk_num);
Hi Stephen:
Thanks for your comments.
On Thu, Jan 9, 2025 at 3:20 AM Stephen Boyd sboyd@kernel.org wrote:
Quoting Binbin Zhou (2025-01-07 17:41:43)
On Wed, Jan 8, 2025 at 5:25 AM Stephen Boyd sboyd@kernel.org wrote:
Quoting Binbin Zhou (2024-12-24 22:05:59)
diff --git a/drivers/clk/clk-loongson2.c b/drivers/clk/clk-loongson2.c index 6bf51d5a49a1..b1b2038acd0b 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);NULL is a valid clk. Either fill the onecell data with -ENOENT error pointers, or stop using it and implement a custom version of of_clk_hw_onecell_get() that doesn't allow invalid clks to be requested from this provider.
Emm... Just in case, how about setting all items to ERR_PTR(-ENOENT) before assigning them. This is shown below:
while (--clk_num >= 0) clp->clk_data.hws[clk_num] = ERR_PTR(-ENOENT);Or something like:
memset_p(&clk->clk_data.hws, ERR_PTR(-ENOENT), clk_num);
Indeed, it looks better and cleaner. I'll update in V2 soon.
-- Thanks. Binbin
linux-stable-mirror@lists.linaro.org
-
Binbin Zhou -
Binbin Zhou -
Stephen Boyd