Skip to contentSkip to conversation

staging: media: atomisp: prefer kcalloc over kzalloc with multiply

PATCH Superseded Andrew Soto 4 participants 7 messages Jun 6 – Jun 8 2 messages not in the archive

Subsystems (4)
Media (V4L/DVB) Staging Intel MID (Mobile Internet Device) platform Staging - atomisp driver
lore
Share & tools
lore

Andrew Soto

author
#0 lore raw

PATCH 1/1 staging: media: atomisp: prefer kcalloc over kzalloc with multiply

Optimize memory allocation layout in sh_css_params.c by replacing the raw multiplication inside kzalloc() with a type-safe kcalloc() array allocation wrapper.

This prevents potential integer overflow vulnerabilities by validating the array size calculations before interacting with the kernel heap allocator, aligning the driver with modern kernel memory allocation standards.

Signed-off-by Andrew Soto <[email protected]>
---
 drivers/staging/media/atomisp/pci/sh_css_params.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/media/atomisp/pci/sh_css_params.c +1 -1
index fcebace11..9147ca047 100644
@@ -3716,7 +3716,7 @@ ia_css_ptr sh_css_store_sp_group_to_ddr(void)
 
 	IA_CSS_ENTER_LEAVE_PRIVATE("void");
 
-	write_buf = kzalloc(sizeof(u8) * 8192, GFP_KERNEL);
+	write_buf = kcalloc(8192, sizeof(u8), GFP_KERNEL);
 	if (!write_buf)
 		return 0;
Signature · 1 line
2.53.0
AI review
#1 lore raw
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Ignored memory allocation failure leads to null-equivalent pointer passed to SP firmware. --

commit 967be0a2dfa51f2c0edc88c4aa3976bc41aab85d
Author: Andrew Soto <[email protected]>

staging: media: atomisp: prefer kcalloc over kzalloc with multiply

This commit replaces a raw multiplication inside kzalloc() with a kcalloc() array allocation in sh_css_params.c. This validates the array size calculations to prevent potential integer overflow vulnerabilities before interacting with the kernel heap allocator.

drivers/staging/media/atomisp/pci/sh_css_params.c ia_css_ptr sh_css_store_sp_group_to_ddr(void) · line 3721

Quoted 12 lines
drivers/staging/media/atomisp/pci/sh_css_params.c +1 -1
index fcebace11daf6..9147ca0474ad2 100644
@@ -3716,7 +3716,7 @@ ia_css_ptr sh_css_store_sp_group_to_ddr(void)
 
 	IA_CSS_ENTER_LEAVE_PRIVATE("void");
 
-	write_buf = kzalloc(sizeof(u8) * 8192, GFP_KERNEL);
+	write_buf = kcalloc(8192, sizeof(u8), GFP_KERNEL);
 	if (!write_buf)
 		return 0;

[Severity: High]
This isn't a bug introduced by this patch, but is it possible that this early return causes the firmware to crash under memory pressure?

If kcalloc() fails, sh_css_store_sp_group_to_ddr() returns 0, which acts as a null-equivalent for an ia_css_ptr.

Looking at the caller in drivers/staging/media/atomisp/pci/sh_css_sp.c:

store_sp_group_data() {
    ...
    per_frame_data.sp_group_addr = sh_css_store_sp_group_to_ddr();
    ...
}

The return value is unconditionally assigned to per_frame_data.sp_group_addr without any validation. When this 0 is sent directly to the per-frame hardware configuration, will the SP firmware crash or lock up the hardware when attempting to access address 0 for its group configuration?

Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
#2 lore raw
In reply to Andrew Soto
Quoted 4 lines · Andrew Soto

Optimize memory allocation layout in sh_css_params.c by replacing the raw multiplication inside kzalloc() with a type-safe kcalloc() array allocation wrapper.

This prevents potential integer overflow vulnerabilities by validating the array size calculations before interacting with the kernel heap allocator, aligning the driver with modern kernel memory allocation standards.

Wrap the commit message around 72 characters per line.

...

Is this the only case like this in the entire driver?

Signature · 2 lines
With Best Regards,
Andy Shevchenko
author
#6 lore raw

PATCH v2 1/1 staging: media: atomisp: prefer kcalloc over kzalloc with multiply

In reply to Andrew Soto

Optimize memory allocation layout in sh_css_params.c by replacing the raw multiplication inside kzalloc() with a type-safe kcalloc() array allocation wrapper.

This prevents potential integer overflow vulnerabilities by validating the array size calculations before interacting
with the kernel heap allocator, aligning the driver with modern kernel memory allocation standards.

Signed-off-by Andrew Soto <[email protected]>
---
 drivers/staging/media/atomisp/pci/sh_css_params.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/media/atomisp/pci/sh_css_params.c +1 -1
index fcebace11..9147ca047 100644
@@ -3716,7 +3716,7 @@ ia_css_ptr sh_css_store_sp_group_to_ddr(void)
 
 	IA_CSS_ENTER_LEAVE_PRIVATE("void");
 
-	write_buf = kzalloc(sizeof(u8) * 8192, GFP_KERNEL);
+	write_buf = kcalloc(8192, sizeof(u8), GFP_KERNEL);
 	if (!write_buf)
 		return 0;
Signature · 1 line
2.53.0
#5 lore raw

PATCH 1/1 staging: media: atomisp: prefer kcalloc over kzalloc with multiply

In reply to a message not in the archive

I just noticed you also replied privately, no, please do the development and communications publicly.

---------- Forwarded message ---------
From: Andy Shevchenko <[email protected]>
Date: Sun, Jun 7, 2026 at 9:48 PM
Subject: Re: [PATCH] staging: media: atomisp: prefer kcalloc over kzalloc with multiply
To: <[email protected]>

Quoted 1 line · On Sun, Jun 7, 2026 at 3:27 PM <[email protected]> wrote:

Yes, I ran grep across the entire atomisp directory and this was the only instance of a raw multiplication inside a standard kzalloc() call. The other files already use kzalloc_obj().

https://elixir.bootlin.com/linux/v7.1-rc6/source/drivers/staging/media/atomisp/pci/sh_css_params.c#L4209
In the same file...
And maybe more, please try again. I suggest using coccinelle to find all the places like this.

Quoted 1 line

I have submitted PATCH V2 with the commit message lines wrapping under 72 characters.

You also have to reply wrapping the lines accordingly and do not top-post!

Quoted 8 lines
Quoted 8 lines · Andy Shevchenko
Quoted 4 lines · Andrew Soto

Optimize memory allocation layout in sh_css_params.c by replacing the raw multiplication inside kzalloc() with a type-safe kcalloc() array allocation wrapper.

This prevents potential integer overflow vulnerabilities by validating the array size calculations before interacting with the kernel heap allocator, aligning the driver with modern kernel memory allocation standards.

Wrap the commit message around 72 characters per line.

...

Quoted 1 line
Quoted 1 line

Is this the only case like this in the entire driver?

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#7 lore raw
In reply to Andrew Soto
Quoted 9 lines · Andrew Soto

Optimize memory allocation layout in sh_css_params.c by replacing the raw multiplication inside kzalloc() with a type-safe kcalloc() array allocation wrapper.

This prevents potential integer overflow vulnerabilities by validating the array size calculations before interacting
with the kernel heap allocator, aligning the driver with modern kernel memory allocation standards.

See my reply to v1.

...

drivers/staging/media/atomisp/pci/sh_css_params.c ia_css_ptr sh_css_store_sp_group_to_ddr(void) · line 3719

Quoted 2 lines
drivers/staging/media/atomisp/pci/sh_css_params.c +1 -1
-       write_buf = kzalloc(sizeof(u8) * 8192, GFP_KERNEL);
+       write_buf = kcalloc(8192, sizeof(u8), GFP_KERNEL);

While at it, you can switch to a more robust sizeof(*write_buf).

Signature · 2 lines
With Best Regards,
Andy Shevchenko
#8 lore raw
In reply to Andrew Soto
Quoted 1 line · Andrew Soto

Optimize memory allocation layout

This part of the commit message is techno-bable. It's just words that don't mean anything. I suppose teally they do mean something, but it's not what the patch does...

Quoted 4 lines

in sh_css_params.c by replacing the raw multiplication inside kzalloc() with a type-safe kcalloc() array allocation wrapper.

This prevents potential integer overflow vulnerabilities by validating the array size calculations before interacting with the kernel heap allocator, aligning the driver with modern kernel memory allocation standards.

There is no risk of integer overflow when we multiply by 1.

Quoted 10 lines
Signed-off-by Andrew Soto <[email protected]>
---
 drivers/staging/media/atomisp/pci/sh_css_params.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/staging/media/atomisp/pci/sh_css_params.c +0 -0
index fcebace11..9147ca047 100644

drivers/staging/media/atomisp/pci/sh_css_params.c ia_css_ptr sh_css_store_sp_group_to_ddr(void) · line 3719

Quoted 5 lines
drivers/staging/media/atomisp/pci/sh_css_params.c +1 -1
@@ -3716,7 +3716,7 @@ ia_css_ptr sh_css_store_sp_group_to_ddr(void)
 
 	IA_CSS_ENTER_LEAVE_PRIVATE("void");
 
-	write_buf = kzalloc(sizeof(u8) * 8192, GFP_KERNEL);
+	write_buf = kcalloc(8192, sizeof(u8), GFP_KERNEL);

This should just be:

	write_buf = kzalloc(8192, GFP_KERNEL);

If we weren't allocating a text buffer then the new way to write this would be using kzalloc_objs().

regards,
dan carpenter