[PATCH] drm/solomon: Use kmalloc_array() instead of kmalloc()

Mehdi Ben Hadj Khelifa posted 1 patch 3 months, 3 weeks ago
drivers/gpu/drm/solomon/ssd130x.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
[PATCH] drm/solomon: Use kmalloc_array() instead of kmalloc()
Posted by Mehdi Ben Hadj Khelifa 3 months, 3 weeks ago
Replace kmalloc() with kmalloc_array() in several places to correctly
handle array allocations and benefit from built-in overflow checking.
This prevents potential integer overflows[1] when computing allocation
sizes from width, height, pitch, or page values.

[1]:https://docs.kernel.org/process/deprecated.html

Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
---
 drivers/gpu/drm/solomon/ssd130x.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/solomon/ssd130x.c b/drivers/gpu/drm/solomon/ssd130x.c
index eec43d1a5595..0ce630861d34 100644
--- a/drivers/gpu/drm/solomon/ssd130x.c
+++ b/drivers/gpu/drm/solomon/ssd130x.c
@@ -1498,7 +1498,7 @@ static int ssd130x_crtc_atomic_check(struct drm_crtc *crtc,
 	if (ret)
 		return ret;
 
-	ssd130x_state->data_array = kmalloc(ssd130x->width * pages, GFP_KERNEL);
+	ssd130x_state->data_array = kmalloc_array(pages, ssd130x->width, GFP_KERNEL);
 	if (!ssd130x_state->data_array)
 		return -ENOMEM;
 
@@ -1519,7 +1519,7 @@ static int ssd132x_crtc_atomic_check(struct drm_crtc *crtc,
 	if (ret)
 		return ret;
 
-	ssd130x_state->data_array = kmalloc(columns * ssd130x->height, GFP_KERNEL);
+	ssd130x_state->data_array = kmalloc_array(ssd130x->height, columns, GFP_KERNEL);
 	if (!ssd130x_state->data_array)
 		return -ENOMEM;
 
@@ -1546,7 +1546,7 @@ static int ssd133x_crtc_atomic_check(struct drm_crtc *crtc,
 
 	pitch = drm_format_info_min_pitch(fi, 0, ssd130x->width);
 
-	ssd130x_state->data_array = kmalloc(pitch * ssd130x->height, GFP_KERNEL);
+	ssd130x_state->data_array = kmalloc_array(ssd130x->height, pitch, GFP_KERNEL);
 	if (!ssd130x_state->data_array)
 		return -ENOMEM;
 
-- 
2.51.1.dirty
Re: [PATCH] drm/solomon: Use kmalloc_array() instead of kmalloc()
Posted by Shuah Khan 3 months, 2 weeks ago
On 10/19/25 08:58, Mehdi Ben Hadj Khelifa wrote:
> Replace kmalloc() with kmalloc_array() in several places to correctly
> handle array allocations and benefit from built-in overflow checking.
> This prevents potential integer overflows[1] when computing allocation
> sizes from width, height, pitch, or page values.
> 
> [1]:https://docs.kernel.org/process/deprecated.html

Mu understanding is that this document lists deprecates APIs so people
don't keep adding new ones.

I didn't get the impression that we are supposed to go delete them from
the kernel and cause a churn.

How are you testing these changes - do you have this hardware?

> 
> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
> ---
>   drivers/gpu/drm/solomon/ssd130x.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 

thanks,
-- Shuah
Re: [PATCH] drm/solomon: Use kmalloc_array() instead of kmalloc()
Posted by Mehdi Ben Hadj Khelifa 3 months, 2 weeks ago
On 10/20/25 9:08 PM, Shuah Khan wrote:
> On 10/19/25 08:58, Mehdi Ben Hadj Khelifa wrote:
>> Replace kmalloc() with kmalloc_array() in several places to correctly
>> handle array allocations and benefit from built-in overflow checking.
>> This prevents potential integer overflows[1] when computing allocation
>> sizes from width, height, pitch, or page values.
>>
>> [1]:https://docs.kernel.org/process/deprecated.html
> 
> Mu understanding is that this document lists deprecates APIs so people
> don't keep adding new ones.
> 
> I didn't get the impression that we are supposed to go delete them from
> the kernel and cause a churn.
> 
the document[1] specifically quotes the following:"
Dynamic size calculations (especially multiplication) should not be 
performed in memory allocator (or similar) function arguments due to the 
risk of them overflowing. This could lead to values wrapping around and 
a smaller allocation being made than the caller was expecting. Using 
those allocations could lead to linear overflows of heap memory and 
other misbehaviors. (One exception to this is literal values where the 
compiler can warn if they might overflow. However, the preferred way in 
these cases is to refactor the code as suggested below to avoid the 
open-coded arithmetic.)"
Specifically mentionned the refactor of the code base in such cases 
which is why i'm doing the patches in the first place.Also i'm trying 
the best to send patches related to the issue where such issues of 
overflow are present or to be consistent with the same API used within 
the same subsystem.
[1]:https://docs.kernel.org/process/deprecated.html> How are you testing 
these changes - do you have this hardware?
> 
>>
I have a raspberrypi zero 2 wh that i'm using in combination with the 
ssd1306 OLED panel via I2C to test it's rendering and it's working 
properly by using modetest and seeing no regressions or warnings in dmesg.

Best Regards,
Mehdi Ben Hadj Khelifa

>> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
>> ---
>>   drivers/gpu/drm/solomon/ssd130x.c | 6 +++---
>>   1 file changed, 3 insertions(+), 3 deletions(-)
>>
> 
> thanks,
> -- Shuah

Re: [PATCH] drm/solomon: Use kmalloc_array() instead of kmalloc()
Posted by Shuah Khan 3 months, 2 weeks ago
On 10/20/25 15:38, Mehdi Ben Hadj Khelifa wrote:
> On 10/20/25 9:08 PM, Shuah Khan wrote:
>> On 10/19/25 08:58, Mehdi Ben Hadj Khelifa wrote:
>>> Replace kmalloc() with kmalloc_array() in several places to correctly
>>> handle array allocations and benefit from built-in overflow checking.
>>> This prevents potential integer overflows[1] when computing allocation
>>> sizes from width, height, pitch, or page values.
>>>
>>> [1]:https://docs.kernel.org/process/deprecated.html
>>
>> Mu understanding is that this document lists deprecates APIs so people
>> don't keep adding new ones.
>>
>> I didn't get the impression that we are supposed to go delete them from
>> the kernel and cause a churn.
>>
> the document[1] specifically quotes the following:"
> Dynamic size calculations (especially multiplication) should not be performed in memory allocator (or similar) function arguments due to the risk of them overflowing. This could lead to values wrapping around and a smaller allocation being made than the caller was expecting. Using those allocations could lead to linear overflows of heap memory and other misbehaviors. (One exception to this is literal values where the compiler can warn if they might overflow. However, the preferred way in these cases is to refactor the code as suggested below to avoid the open-coded arithmetic.)"
> Specifically mentionned the refactor of the code base in such cases which is why i'm doing the patches in the first place.Also i'm trying the best to send patches related to the issue where such issues of overflow are present or to be consistent with the same API used within the same subsystem.
> [1]:https://docs.kernel.org/process/deprecated.html> How are you testing these changes - do you have this hardware?
>>
>>>
> I have a raspberrypi zero 2 wh that i'm using in combination with the ssd1306 OLED panel via I2C to test it's rendering and it's working properly by using modetest and seeing no regressions or warnings in dmesg.
> 

Send v2 with all these details and why this change is needed
in the first place.

When and how does this potential problem trigger? Is this a
theoretical or does this happen in this code path and how?
Next time include all of these details people understand the
problem better.

thanks,
-- Shuah
Re: [PATCH] drm/solomon: Use kmalloc_array() instead of kmalloc()
Posted by Mehdi Ben Hadj Khelifa 3 months, 2 weeks ago
On 10/20/25 9:56 PM, Shuah Khan wrote:
> On 10/20/25 15:38, Mehdi Ben Hadj Khelifa wrote:
>> On 10/20/25 9:08 PM, Shuah Khan wrote:
>>> On 10/19/25 08:58, Mehdi Ben Hadj Khelifa wrote:
>>>> Replace kmalloc() with kmalloc_array() in several places to correctly
>>>> handle array allocations and benefit from built-in overflow checking.
>>>> This prevents potential integer overflows[1] when computing allocation
>>>> sizes from width, height, pitch, or page values.
>>>>
>>>> [1]:https://docs.kernel.org/process/deprecated.html
>>>
>>> Mu understanding is that this document lists deprecates APIs so people
>>> don't keep adding new ones.
>>>
>>> I didn't get the impression that we are supposed to go delete them from
>>> the kernel and cause a churn.
>>>
>> the document[1] specifically quotes the following:"
>> Dynamic size calculations (especially multiplication) should not be 
>> performed in memory allocator (or similar) function arguments due to 
>> the risk of them overflowing. This could lead to values wrapping 
>> around and a smaller allocation being made than the caller was 
>> expecting. Using those allocations could lead to linear overflows of 
>> heap memory and other misbehaviors. (One exception to this is literal 
>> values where the compiler can warn if they might overflow. However, 
>> the preferred way in these cases is to refactor the code as suggested 
>> below to avoid the open-coded arithmetic.)"
>> Specifically mentionned the refactor of the code base in such cases 
>> which is why i'm doing the patches in the first place.Also i'm trying 
>> the best to send patches related to the issue where such issues of 
>> overflow are present or to be consistent with the same API used within 
>> the same subsystem.
>> [1]:https://docs.kernel.org/process/deprecated.html> How are you 
>> testing these changes - do you have this hardware?
>>>
>>>>
>> I have a raspberrypi zero 2 wh that i'm using in combination with the 
>> ssd1306 OLED panel via I2C to test it's rendering and it's working 
>> properly by using modetest and seeing no regressions or warnings in 
>> dmesg.
>>
> 
> Send v2 with all these details and why this change is needed
> in the first place.
> 
Okay, I will do that as soon as possible.> When and how does this 
potential problem trigger? Is this a
> theoretical or does this happen in this code path and how?
> Next time include all of these details people understand the
> problem better.
> 
We'll do in the next iteration.Thanks

BR,
Mehdi> thanks,
> -- Shuah
>
Re: [PATCH] drm/solomon: Use kmalloc_array() instead of kmalloc()
Posted by Javier Martinez Canillas 3 months, 2 weeks ago
Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com> writes:

Hello Mehdi,

> On 10/20/25 9:56 PM, Shuah Khan wrote:

[...]

>>> I have a raspberrypi zero 2 wh that i'm using in combination with the 
>>> ssd1306 OLED panel via I2C to test it's rendering and it's working 
>>> properly by using modetest and seeing no regressions or warnings in 
>>> dmesg.
>>>
>> 
>> Send v2 with all these details and why this change is needed
>> in the first place.
>> 
> Okay, I will do that as soon as possible.> When and how does this 
> potential problem trigger? Is this a
>> theoretical or does this happen in this code path and how?
>> Next time include all of these details people understand the
>> problem better.
>> 
> We'll do in the next iteration.Thanks
>

A similar patch was posted by another developer a couple of weeks
ago and is now queued already in the drm-misc-next branch:

https://cgit.freedesktop.org/drm/drm-misc/commit/?id=940dd88c5f5bdb1f3e19873a856a677ebada63a9

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat
Re: [PATCH] drm/solomon: Use kmalloc_array() instead of kmalloc()
Posted by Mehdi Ben Hadj Khelifa 3 months, 2 weeks ago
On 10/21/25 8:51 AM, Javier Martinez Canillas wrote:
> Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com> writes:
> 
> Hello Mehdi,
> 
>> On 10/20/25 9:56 PM, Shuah Khan wrote:
> 
> [...]
> 
>>>> I have a raspberrypi zero 2 wh that i'm using in combination with the
>>>> ssd1306 OLED panel via I2C to test it's rendering and it's working
>>>> properly by using modetest and seeing no regressions or warnings in
>>>> dmesg.
>>>>
>>>
>>> Send v2 with all these details and why this change is needed
>>> in the first place.
>>>
>> Okay, I will do that as soon as possible.> When and how does this
>> potential problem trigger? Is this a
>>> theoretical or does this happen in this code path and how?
>>> Next time include all of these details people understand the
>>> problem better.
>>>
>> We'll do in the next iteration.Thanks
>>
> 
> A similar patch was posted by another developer a couple of weeks
> ago and is now queued already in the drm-misc-next branch:
> 
> https://cgit.freedesktop.org/drm/drm-misc/commit/?id=940dd88c5f5bdb1f3e19873a856a677ebada63a9
> 
Hello javier,

Thanks for the heads up. I will abort working on the patch.

Best Regards,
Mehdi Ben Hadj Khelifa