init/initramfs_test.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
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
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.
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
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
© 2016 - 2025 Red Hat, Inc.