[PATCH] sched/topology: Use kcalloc() in sched_init_numa()

Qianfeng Rong posted 1 patch 1 month, 1 week ago
There is a newer version of this series
kernel/sched/topology.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
[PATCH] sched/topology: Use kcalloc() in sched_init_numa()
Posted by Qianfeng Rong 1 month, 1 week ago
Replace kzalloc() with kcalloc() in sched_init_numa().  As noted in
the kernel documentation [1], open-coded multiplication in allocator
arguments is discouraged because it can lead to integer overflow.

Use kcalloc() to gain built-in overflow protection, making memory
allocation safer when calculating allocation size compared to explicit
multiplication. Similarly, use size_add() instead of explicit addition
for 'i + nr_levels + 1'.

Link: https://www.kernel.org/doc/html/next/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments #1
Signed-off-by: Qianfeng Rong <rongqianfeng@vivo.com>
---
 kernel/sched/topology.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/sched/topology.c b/kernel/sched/topology.c
index 977e133bb8a4..0500146f9c1f 100644
--- a/kernel/sched/topology.c
+++ b/kernel/sched/topology.c
@@ -1956,7 +1956,7 @@ void sched_init_numa(int offline_node)
 	 */
 	sched_domains_numa_levels = 0;
 
-	masks = kzalloc(sizeof(void *) * nr_levels, GFP_KERNEL);
+	masks = kcalloc(nr_levels, sizeof(void *), GFP_KERNEL);
 	if (!masks)
 		return;
 
@@ -1965,7 +1965,7 @@ void sched_init_numa(int offline_node)
 	 * CPUs of nodes that are that many hops away from us.
 	 */
 	for (i = 0; i < nr_levels; i++) {
-		masks[i] = kzalloc(nr_node_ids * sizeof(void *), GFP_KERNEL);
+		masks[i] = kcalloc(nr_node_ids, sizeof(void *), GFP_KERNEL);
 		if (!masks[i])
 			return;
 
@@ -1994,8 +1994,8 @@ void sched_init_numa(int offline_node)
 	/* Compute default topology size */
 	for (i = 0; sched_domain_topology[i].mask; i++);
 
-	tl = kzalloc((i + nr_levels + 1) *
-			sizeof(struct sched_domain_topology_level), GFP_KERNEL);
+	tl = kcalloc(size_add(size_add(i, nr_levels), 1),
+		     sizeof(struct sched_domain_topology_level), GFP_KERNEL);
 	if (!tl)
 		return;
 
-- 
2.34.1
Re: [PATCH] sched/topology: Use kcalloc() in sched_init_numa()
Posted by Valentin Schneider 1 month ago
On 21/08/25 22:08, Qianfeng Rong wrote:
> Replace kzalloc() with kcalloc() in sched_init_numa().  As noted in
> the kernel documentation [1], open-coded multiplication in allocator
> arguments is discouraged because it can lead to integer overflow.
>

Hm, checkpatch does warn about these.

> Use kcalloc() to gain built-in overflow protection, making memory
> allocation safer when calculating allocation size compared to explicit
> multiplication. Similarly, use size_add() instead of explicit addition
> for 'i + nr_levels + 1'.
>
> Link:
> https://www.kernel.org/doc/html/next/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments
> #1

This document is actually in the kernel sources, it would be better to
reference it as:

  Documentation/process/deprecated.rst

> @@ -1994,8 +1994,8 @@ void sched_init_numa(int offline_node)
>       /* Compute default topology size */
>       for (i = 0; sched_domain_topology[i].mask; i++);
>
> -	tl = kzalloc((i + nr_levels + 1) *
> -			sizeof(struct sched_domain_topology_level), GFP_KERNEL);
> +	tl = kcalloc(size_add(size_add(i, nr_levels), 1),
> +		     sizeof(struct sched_domain_topology_level), GFP_KERNEL);

@nr_levels is at most 256 (including illegal values between 0 and 9); @i is
going to be the size of sched_domain_topology[]; is the double size_add()
really warranted here?

>       if (!tl)
>               return;
>
> --
> 2.34.1
Re: [PATCH] sched/topology: Use kcalloc() in sched_init_numa()
Posted by Qianfeng Rong 1 month ago
在 2025/8/28 19:11, Valentin Schneider 写道:
> On 21/08/25 22:08, Qianfeng Rong wrote:
>> Replace kzalloc() with kcalloc() in sched_init_numa().  As noted in
>> the kernel documentation [1], open-coded multiplication in allocator
>> arguments is discouraged because it can lead to integer overflow.
>>
> Hm, checkpatch does warn about these.
>
>> Use kcalloc() to gain built-in overflow protection, making memory
>> allocation safer when calculating allocation size compared to explicit
>> multiplication. Similarly, use size_add() instead of explicit addition
>> for 'i + nr_levels + 1'.
>>
>> Link:
>> https://www.kernel.org/doc/html/next/process/deprecated.html#open-coded-arithmetic-in-allocator-arguments
>> #1
> This document is actually in the kernel sources, it would be better to
> reference it as:
>
>    Documentation/process/deprecated.rst
I didn't notice this, thank you for the reminder.
>
>> @@ -1994,8 +1994,8 @@ void sched_init_numa(int offline_node)
>>        /* Compute default topology size */
>>        for (i = 0; sched_domain_topology[i].mask; i++);
>>
>> -	tl = kzalloc((i + nr_levels + 1) *
>> -			sizeof(struct sched_domain_topology_level), GFP_KERNEL);
>> +	tl = kcalloc(size_add(size_add(i, nr_levels), 1),
>> +		     sizeof(struct sched_domain_topology_level), GFP_KERNEL);
> @nr_levels is at most 256 (including illegal values between 0 and 9); @i is
> going to be the size of sched_domain_topology[]; is the double size_add()
> really warranted here?


You're right that the maximum value of nr_levels is fixed, and the double
size_add() to size_add() here are indeed unnecessary. I will submit the v2
version.

Best regards,
Qianfeng