[PATCH] init: Use kcalloc() instead of kzalloc()

Mehdi Ben Hadj Khelifa posted 1 patch 2 months, 2 weeks ago
init/initramfs_test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] init: Use kcalloc() instead of kzalloc()
Posted by Mehdi Ben Hadj Khelifa 2 months, 2 weeks ago
Replace kzalloc() with kcalloc() in init/initramfs_test.c since the
calculation inside kzalloc is dynamic and could overflow.

Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
---
 init/initramfs_test.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/init/initramfs_test.c b/init/initramfs_test.c
index 517e5e04e5cc..fed1911b73cb 100644
--- a/init/initramfs_test.c
+++ b/init/initramfs_test.c
@@ -97,7 +97,7 @@ static void __init initramfs_test_extract(struct kunit *test)
 	} };
 
 	/* +3 to cater for any 4-byte end-alignment */
-	cpio_srcbuf = kzalloc(ARRAY_SIZE(c) * (CPIO_HDRLEN + PATH_MAX + 3),
+	cpio_srcbuf = kcalloc(ARRAY_SIZE(c), (CPIO_HDRLEN + PATH_MAX + 3),
 			      GFP_KERNEL);
 	len = fill_cpio(c, ARRAY_SIZE(c), cpio_srcbuf);
 
-- 
2.51.0
Re: [PATCH] init: Use kcalloc() instead of kzalloc()
Posted by Al Viro 2 months, 2 weeks ago
On Tue, Sep 30, 2025 at 09:35:37AM +0100, Mehdi Ben Hadj Khelifa wrote:
> Replace kzalloc() with kcalloc() in init/initramfs_test.c since the
> calculation inside kzalloc is dynamic and could overflow.

Really?  Could you explain how
	a) ARRAY_SIZE(local variable) * (CPIO_HDRLEN + PATH_MAX + 3)
could possibly be dynamic and
	b) just how large would that array have to be for it to "overflow"?

Incidentally, here the use of kcalloc would be unidiomatic - it's _not_
allocating an array of that many fixed-sized elements.  CPIO_HDRLEN +
PATH_MAX + 3 is not an element size - it's an upper bound on the amount
of space we might need for a single element.  Chunks of data generated
from array elements are placed into that buffer without any gaps -
it's really an array of bytes, large enough to fit all of them.
Re: [PATCH] init: Use kcalloc() instead of kzalloc()
Posted by Mehdi Ben Hadj Khelifa 2 months, 2 weeks ago
On 10/2/25 3:36 AM, Al Viro wrote:
> On Tue, Sep 30, 2025 at 09:35:37AM +0100, Mehdi Ben Hadj Khelifa wrote:
>> Replace kzalloc() with kcalloc() in init/initramfs_test.c since the
>> calculation inside kzalloc is dynamic and could overflow.
> 
> Really?  Could you explain how
> 	a) ARRAY_SIZE(local variable) * (CPIO_HDRLEN + PATH_MAX + 3)
> could possibly be dynamic and
I missed that c is in local scope.It's already of size 3 and since 
CPIO_HDRLEN is 110 and PATH_MAX is 4096 + 3, it's far from the limit and 
it is calculated at compile time since all values are deducible.> 	b) 
just how large would that array have to be for it to "overflow"?
If c could be of any size, it would have to be of size 1,020,310 for 
32-bit kernels and a lot for 64-bit kernels around 4.4 quadrillion 
elements. Which is unrealistic.

> Incidentally, here the use of kcalloc would be unidiomatic - it's _not_
> allocating an array of that many fixed-sized elements.  CPIO_HDRLEN +
> PATH_MAX + 3 is not an element size - it's an upper bound on the amount
> of space we might need for a single element.  Chunks of data generated
> from array elements are placed into that buffer without any gaps -
> it's really an array of bytes, large enough to fit all of them.
Yes I get it now. But Even if the CPIO_HDRLEN + PATH_MAX + 3 is the 
upper bound on the amount of space and in use it doesn't have any gaps 
in memory, Shouldn't we change kzalloc() to kcalloc() since kzalloc() is 
deprecated[1]?
Regards,
Mehdi Ben Hadj Khelifa

[1]:https://docs.kernel.org/process/deprecated.html
Re: [PATCH] init: Use kcalloc() instead of kzalloc()
Posted by Mehdi Ben Hadj Khelifa 1 month, 4 weeks ago
On 10/3/25 11:40 AM, Mehdi Ben Hadj Khelifa wrote:
> On 10/2/25 3:36 AM, Al Viro wrote:
>> On Tue, Sep 30, 2025 at 09:35:37AM +0100, Mehdi Ben Hadj Khelifa wrote:
>>> Replace kzalloc() with kcalloc() in init/initramfs_test.c since the
>>> calculation inside kzalloc is dynamic and could overflow.
>>
>> Really?  Could you explain how
>>     a) ARRAY_SIZE(local variable) * (CPIO_HDRLEN + PATH_MAX + 3)
>> could possibly be dynamic and
> I missed that c is in local scope.It's already of size 3 and since 
> CPIO_HDRLEN is 110 and PATH_MAX is 4096 + 3, it's far from the limit and 
> it is calculated at compile time since all values are deducible.>     b) 
> just how large would that array have to be for it to "overflow"?
> If c could be of any size, it would have to be of size 1,020,310 for 32- 
> bit kernels and a lot for 64-bit kernels around 4.4 quadrillion 
> elements. Which is unrealistic.
> 
>> Incidentally, here the use of kcalloc would be unidiomatic - it's _not_
>> allocating an array of that many fixed-sized elements.  CPIO_HDRLEN +
>> PATH_MAX + 3 is not an element size - it's an upper bound on the amount
>> of space we might need for a single element.  Chunks of data generated
>> from array elements are placed into that buffer without any gaps -
>> it's really an array of bytes, large enough to fit all of them.
> Yes I get it now. But Even if the CPIO_HDRLEN + PATH_MAX + 3 is the 
> upper bound on the amount of space and in use it doesn't have any gaps 
> in memory, Shouldn't we change kzalloc() to kcalloc() since kzalloc() is 
> deprecated[1]?
> Regards,
> Mehdi Ben Hadj Khelifa
> 
> [1]:https://docs.kernel.org/process/deprecated.html

Hello viro,
I'm just resending reply in case if you missed it.

Best regards,
Mehdi Ben Hadj Khelifa