[v1] drm/tiny: Use kmalloc_array() instead of kmalloc()

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

Posted by Mehdi Ben Hadj Khelifa 1 month, 4 weeks ago

Replace kmalloc() with kmalloc_array() to correctly
handle array allocations and benefit from built-in overflow checking[1].

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

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

diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
index 4824f863fdba..290132c24ff9 100644
--- a/drivers/gpu/drm/tiny/repaper.c
+++ b/drivers/gpu/drm/tiny/repaper.c
@@ -534,7 +534,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb, const struct iosys_map *
 	DRM_DEBUG("Flushing [FB:%d] st=%ums\n", fb->base.id,
 		  epd->factored_stage_time);
 
-	buf = kmalloc(fb->width * fb->height / 8, GFP_KERNEL);
+	buf = kmalloc_array(fb->height / 8, fb->width, GFP_KERNEL);
 	if (!buf) {
 		ret = -ENOMEM;
 		goto out_exit;
-- 
2.51.1.dirty

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

Posted by Greg KH 1 month, 4 weeks ago

On Sun, Oct 19, 2025 at 04:12:28PM +0100, Mehdi Ben Hadj Khelifa wrote:
> Replace kmalloc() with kmalloc_array() to correctly
> handle array allocations and benefit from built-in overflow checking[1].
> 
> [1]:https://docs.kernel.org/process/deprecated.html
> 
> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
> ---
>  drivers/gpu/drm/tiny/repaper.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
> index 4824f863fdba..290132c24ff9 100644
> --- a/drivers/gpu/drm/tiny/repaper.c
> +++ b/drivers/gpu/drm/tiny/repaper.c
> @@ -534,7 +534,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb, const struct iosys_map *
>  	DRM_DEBUG("Flushing [FB:%d] st=%ums\n", fb->base.id,
>  		  epd->factored_stage_time);
>  
> -	buf = kmalloc(fb->width * fb->height / 8, GFP_KERNEL);
> +	buf = kmalloc_array(fb->height / 8, fb->width, GFP_KERNEL);

This isn't an array, so this function change doesn't seem to make much
sense, right?  The size should have already been checked earlier in the
call change to be correct.

thanks,

greg k-h

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

Posted by Thomas Zimmermann 1 month, 4 weeks ago

Hi

Am 19.10.25 um 16:34 schrieb Greg KH:
> On Sun, Oct 19, 2025 at 04:12:28PM +0100, Mehdi Ben Hadj Khelifa wrote:
>> Replace kmalloc() with kmalloc_array() to correctly
>> handle array allocations and benefit from built-in overflow checking[1].
>>
>> [1]:https://docs.kernel.org/process/deprecated.html
>>
>> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
>> ---
>>   drivers/gpu/drm/tiny/repaper.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/repaper.c
>> index 4824f863fdba..290132c24ff9 100644
>> --- a/drivers/gpu/drm/tiny/repaper.c
>> +++ b/drivers/gpu/drm/tiny/repaper.c
>> @@ -534,7 +534,7 @@ static int repaper_fb_dirty(struct drm_framebuffer *fb, const struct iosys_map *
>>   	DRM_DEBUG("Flushing [FB:%d] st=%ums\n", fb->base.id,
>>   		  epd->factored_stage_time);
>>   
>> -	buf = kmalloc(fb->width * fb->height / 8, GFP_KERNEL);
>> +	buf = kmalloc_array(fb->height / 8, fb->width, GFP_KERNEL);
> This isn't an array, so this function change doesn't seem to make much
> sense, right?  The size should have already been checked earlier in the
> call change to be correct.

Yes, we've recently received plenty of these pointless changes. The 
correct code would compute the number of bytes per pixel using 
drm_format_info_min_pitch() and multiply with fb->height. The latter 
could (maybe) use kmalloc_array(). It would still not be an array in the 
common sense.

Best regards
Thomas

>
> thanks,
>
> greg k-h

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)

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

Posted by Mehdi Ben Hadj Khelifa 1 month, 4 weeks ago

On 10/19/25 3:47 PM, Thomas Zimmermann wrote:
> Hi
> 
> Am 19.10.25 um 16:34 schrieb Greg KH:
>> On Sun, Oct 19, 2025 at 04:12:28PM +0100, Mehdi Ben Hadj Khelifa wrote:
>>> Replace kmalloc() with kmalloc_array() to correctly
>>> handle array allocations and benefit from built-in overflow checking[1].
>>>
>>> [1]:https://docs.kernel.org/process/deprecated.html
>>>
>>> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
>>> ---
>>>   drivers/gpu/drm/tiny/repaper.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/ 
>>> repaper.c
>>> index 4824f863fdba..290132c24ff9 100644
>>> --- a/drivers/gpu/drm/tiny/repaper.c
>>> +++ b/drivers/gpu/drm/tiny/repaper.c
>>> @@ -534,7 +534,7 @@ static int repaper_fb_dirty(struct 
>>> drm_framebuffer *fb, const struct iosys_map *
>>>       DRM_DEBUG("Flushing [FB:%d] st=%ums\n", fb->base.id,
>>>             epd->factored_stage_time);
>>> -    buf = kmalloc(fb->width * fb->height / 8, GFP_KERNEL);
>>> +    buf = kmalloc_array(fb->height / 8, fb->width, GFP_KERNEL);
>> This isn't an array, so this function change doesn't seem to make much
>> sense, right?  The size should have already been checked earlier in the
>> call change to be correct.
Yes,I was intending to say framebuffer but I was working on another 
similar patch simultaneously so I reused same words by mistake. Thanks 
for clarifying that.>
> Yes, we've recently received plenty of these pointless changes. The 
> correct code would compute the number of bytes per pixel using 
> drm_format_info_min_pitch() and multiply with fb->height. The latter 
> could (maybe) use kmalloc_array(). It would still not be an array in the 
> common sense.
> 
Thanks for the review and suggestion.I will be sending a v2 patch with 
the recommended code change.

Best Regards,
Mehdi Ben Hadj Khelifa> Best regards
> Thomas
> 
>>
>> thanks,
>>
>> greg k-h
> 
> -- 
> -- 
> Thomas Zimmermann
> Graphics Driver Developer
> SUSE Software Solutions Germany GmbH
> Frankenstrasse 146, 90461 Nuernberg, Germany
> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
> HRB 36809 (AG Nuernberg)

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

Posted by Jani Nikula 1 month, 4 weeks ago

On Sun, 19 Oct 2025, Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com> wrote:
> On 10/19/25 3:47 PM, Thomas Zimmermann wrote:
>> Hi
>> 
>> Am 19.10.25 um 16:34 schrieb Greg KH:
>>> On Sun, Oct 19, 2025 at 04:12:28PM +0100, Mehdi Ben Hadj Khelifa wrote:
>>>> Replace kmalloc() with kmalloc_array() to correctly
>>>> handle array allocations and benefit from built-in overflow checking[1].
>>>>
>>>> [1]:https://docs.kernel.org/process/deprecated.html
>>>>
>>>> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
>>>> ---
>>>>   drivers/gpu/drm/tiny/repaper.c | 2 +-
>>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/ 
>>>> repaper.c
>>>> index 4824f863fdba..290132c24ff9 100644
>>>> --- a/drivers/gpu/drm/tiny/repaper.c
>>>> +++ b/drivers/gpu/drm/tiny/repaper.c
>>>> @@ -534,7 +534,7 @@ static int repaper_fb_dirty(struct 
>>>> drm_framebuffer *fb, const struct iosys_map *
>>>>       DRM_DEBUG("Flushing [FB:%d] st=%ums\n", fb->base.id,
>>>>             epd->factored_stage_time);
>>>> -    buf = kmalloc(fb->width * fb->height / 8, GFP_KERNEL);
>>>> +    buf = kmalloc_array(fb->height / 8, fb->width, GFP_KERNEL);

Also worth emphasizing that this is wildly wrong for any height that is
not a multiple of 8.

And I thought I shot down a similar patch not long ago.

Is there some tool that suggests doing this? Fix the tool instead
please.

BR,
Jani.




>>> This isn't an array, so this function change doesn't seem to make much
>>> sense, right?  The size should have already been checked earlier in the
>>> call change to be correct.
> Yes,I was intending to say framebuffer but I was working on another 
> similar patch simultaneously so I reused same words by mistake. Thanks 
> for clarifying that.>
>> Yes, we've recently received plenty of these pointless changes. The 
>> correct code would compute the number of bytes per pixel using 
>> drm_format_info_min_pitch() and multiply with fb->height. The latter 
>> could (maybe) use kmalloc_array(). It would still not be an array in the 
>> common sense.
>> 
> Thanks for the review and suggestion.I will be sending a v2 patch with 
> the recommended code change.
>
> Best Regards,
> Mehdi Ben Hadj Khelifa> Best regards
>> Thomas
>> 
>>>
>>> thanks,
>>>
>>> greg k-h
>> 
>> -- 
>> -- 
>> Thomas Zimmermann
>> Graphics Driver Developer
>> SUSE Software Solutions Germany GmbH
>> Frankenstrasse 146, 90461 Nuernberg, Germany
>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>> HRB 36809 (AG Nuernberg)
>

-- 
Jani Nikula, Intel

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

Posted by Shuah Khan 1 month, 3 weeks ago

On 10/20/25 03:50, Jani Nikula wrote:
> On Sun, 19 Oct 2025, Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com> wrote:
>> On 10/19/25 3:47 PM, Thomas Zimmermann wrote:
>>> Hi
>>>
>>> Am 19.10.25 um 16:34 schrieb Greg KH:
>>>> On Sun, Oct 19, 2025 at 04:12:28PM +0100, Mehdi Ben Hadj Khelifa wrote:
>>>>> Replace kmalloc() with kmalloc_array() to correctly
>>>>> handle array allocations and benefit from built-in overflow checking[1].
>>>>>
>>>>> [1]:https://docs.kernel.org/process/deprecated.html
>>>>>
>>>>> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
>>>>> ---
>>>>>    drivers/gpu/drm/tiny/repaper.c | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/
>>>>> repaper.c
>>>>> index 4824f863fdba..290132c24ff9 100644
>>>>> --- a/drivers/gpu/drm/tiny/repaper.c
>>>>> +++ b/drivers/gpu/drm/tiny/repaper.c
>>>>> @@ -534,7 +534,7 @@ static int repaper_fb_dirty(struct
>>>>> drm_framebuffer *fb, const struct iosys_map *
>>>>>        DRM_DEBUG("Flushing [FB:%d] st=%ums\n", fb->base.id,
>>>>>              epd->factored_stage_time);
>>>>> -    buf = kmalloc(fb->width * fb->height / 8, GFP_KERNEL);
>>>>> +    buf = kmalloc_array(fb->height / 8, fb->width, GFP_KERNEL);
> 
> Also worth emphasizing that this is wildly wrong for any height that is
> not a multiple of 8.
> 
> And I thought I shot down a similar patch not long ago.
> 
> Is there some tool that suggests doing this? Fix the tool instead
> please.
> 

They are documented in 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.

thanks,
-- Shuah

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

Posted by Mehdi Ben Hadj Khelifa 1 month, 3 weeks ago

On 10/20/25 9:06 PM, Shuah Khan wrote:
> On 10/20/25 03:50, Jani Nikula wrote:
>> On Sun, 19 Oct 2025, Mehdi Ben Hadj Khelifa 
>> <mehdi.benhadjkhelifa@gmail.com> wrote:
>>> On 10/19/25 3:47 PM, Thomas Zimmermann wrote:
>>>> Hi
>>>>
>>>> Am 19.10.25 um 16:34 schrieb Greg KH:
>>>>> On Sun, Oct 19, 2025 at 04:12:28PM +0100, Mehdi Ben Hadj Khelifa 
>>>>> wrote:
>>>>>> Replace kmalloc() with kmalloc_array() to correctly
>>>>>> handle array allocations and benefit from built-in overflow 
>>>>>> checking[1].
>>>>>>
>>>>>> [1]:https://docs.kernel.org/process/deprecated.html
>>>>>>
>>>>>> Signed-off-by: Mehdi Ben Hadj Khelifa 
>>>>>> <mehdi.benhadjkhelifa@gmail.com>
>>>>>> ---
>>>>>>    drivers/gpu/drm/tiny/repaper.c | 2 +-
>>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/
>>>>>> repaper.c
>>>>>> index 4824f863fdba..290132c24ff9 100644
>>>>>> --- a/drivers/gpu/drm/tiny/repaper.c
>>>>>> +++ b/drivers/gpu/drm/tiny/repaper.c
>>>>>> @@ -534,7 +534,7 @@ static int repaper_fb_dirty(struct
>>>>>> drm_framebuffer *fb, const struct iosys_map *
>>>>>>        DRM_DEBUG("Flushing [FB:%d] st=%ums\n", fb->base.id,
>>>>>>              epd->factored_stage_time);
>>>>>> -    buf = kmalloc(fb->width * fb->height / 8, GFP_KERNEL);
>>>>>> +    buf = kmalloc_array(fb->height / 8, fb->width, GFP_KERNEL);
>>
>> Also worth emphasizing that this is wildly wrong for any height that is
>> not a multiple of 8.
>>
>> And I thought I shot down a similar patch not long ago.
>>
>> Is there some tool that suggests doing this? Fix the tool instead
>> please.
>>
> 
> They are documented in 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.
> 
I have sent an appropriate v2 specifically to suit the case that we have 
here. But 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

Best Regards,
Mehdi Ben Hadj Khelifa> thanks,
> -- Shuah

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

Posted by Shuah Khan 1 month, 3 weeks ago

On 10/20/25 15:11, Mehdi Ben Hadj Khelifa wrote:
> On 10/20/25 9:06 PM, Shuah Khan wrote:
>> On 10/20/25 03:50, Jani Nikula wrote:
>>> On Sun, 19 Oct 2025, Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com> wrote:
>>>> On 10/19/25 3:47 PM, Thomas Zimmermann wrote:
>>>>> Hi
>>>>>
>>>>> Am 19.10.25 um 16:34 schrieb Greg KH:
>>>>>> On Sun, Oct 19, 2025 at 04:12:28PM +0100, Mehdi Ben Hadj Khelifa wrote:
>>>>>>> Replace kmalloc() with kmalloc_array() to correctly
>>>>>>> handle array allocations and benefit from built-in overflow checking[1].
>>>>>>>
>>>>>>> [1]:https://docs.kernel.org/process/deprecated.html
>>>>>>>
>>>>>>> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
>>>>>>> ---
>>>>>>>    drivers/gpu/drm/tiny/repaper.c | 2 +-
>>>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/
>>>>>>> repaper.c
>>>>>>> index 4824f863fdba..290132c24ff9 100644
>>>>>>> --- a/drivers/gpu/drm/tiny/repaper.c
>>>>>>> +++ b/drivers/gpu/drm/tiny/repaper.c
>>>>>>> @@ -534,7 +534,7 @@ static int repaper_fb_dirty(struct
>>>>>>> drm_framebuffer *fb, const struct iosys_map *
>>>>>>>        DRM_DEBUG("Flushing [FB:%d] st=%ums\n", fb->base.id,
>>>>>>>              epd->factored_stage_time);
>>>>>>> -    buf = kmalloc(fb->width * fb->height / 8, GFP_KERNEL);
>>>>>>> +    buf = kmalloc_array(fb->height / 8, fb->width, GFP_KERNEL);
>>>
>>> Also worth emphasizing that this is wildly wrong for any height that is
>>> not a multiple of 8.
>>>
>>> And I thought I shot down a similar patch not long ago.
>>>
>>> Is there some tool that suggests doing this? Fix the tool instead
>>> please.
>>>
>>
>> They are documented in 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.
>>
> I have sent an appropriate v2 specifically to suit the case that we have here. But 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? Next time give more details on the
where you found the problem - it is easy to miss the link unless you
state that it is coming from the deprecated document.

Even so you have to explain why the change is applicable to the code
you are changing. How are you testing these changes. I have seen more
patches from you in drm drivers and lib code.

thanks,
-- Shuah

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

Posted by Shuah Khan 1 month, 3 weeks ago

On 10/20/25 15:11, Mehdi Ben Hadj Khelifa wrote:
> On 10/20/25 9:06 PM, Shuah Khan wrote:
>> On 10/20/25 03:50, Jani Nikula wrote:
>>> On Sun, 19 Oct 2025, Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com> wrote:
>>>> On 10/19/25 3:47 PM, Thomas Zimmermann wrote:
>>>>> Hi
>>>>>
>>>>> Am 19.10.25 um 16:34 schrieb Greg KH:
>>>>>> On Sun, Oct 19, 2025 at 04:12:28PM +0100, Mehdi Ben Hadj Khelifa wrote:
>>>>>>> Replace kmalloc() with kmalloc_array() to correctly
>>>>>>> handle array allocations and benefit from built-in overflow checking[1].
>>>>>>>
>>>>>>> [1]:https://docs.kernel.org/process/deprecated.html
>>>>>>>
>>>>>>> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
>>>>>>> ---
>>>>>>>    drivers/gpu/drm/tiny/repaper.c | 2 +-
>>>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/
>>>>>>> repaper.c
>>>>>>> index 4824f863fdba..290132c24ff9 100644
>>>>>>> --- a/drivers/gpu/drm/tiny/repaper.c
>>>>>>> +++ b/drivers/gpu/drm/tiny/repaper.c
>>>>>>> @@ -534,7 +534,7 @@ static int repaper_fb_dirty(struct
>>>>>>> drm_framebuffer *fb, const struct iosys_map *
>>>>>>>        DRM_DEBUG("Flushing [FB:%d] st=%ums\n", fb->base.id,
>>>>>>>              epd->factored_stage_time);
>>>>>>> -    buf = kmalloc(fb->width * fb->height / 8, GFP_KERNEL);
>>>>>>> +    buf = kmalloc_array(fb->height / 8, fb->width, GFP_KERNEL);
>>>
>>> Also worth emphasizing that this is wildly wrong for any height that is
>>> not a multiple of 8.
>>>
>>> And I thought I shot down a similar patch not long ago.
>>>
>>> Is there some tool that suggests doing this? Fix the tool instead
>>> please.
>>>
>>
>> They are documented in 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.
>>
> I have sent an appropriate v2 specifically to suit the case that we have here. But 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? Next time give more details on the
where you found the problem - it is easy to miss the link unless you
state that it is coming from the deprecated document.

Even so you have to explain why the change is applicable to the code
you are changing. How are you testing these changes. I have seen more
patches from you in drm drivers and lib code.

thanks,
-- Shuah

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

Posted by Mehdi Ben Hadj Khelifa 1 month, 3 weeks ago

On 10/20/25 9:22 PM, Shuah Khan wrote:
> On 10/20/25 15:11, Mehdi Ben Hadj Khelifa wrote:
>> On 10/20/25 9:06 PM, Shuah Khan wrote:
>>> On 10/20/25 03:50, Jani Nikula wrote:
>>>> On Sun, 19 Oct 2025, Mehdi Ben Hadj Khelifa 
>>>> <mehdi.benhadjkhelifa@gmail.com> wrote:
>>>>> On 10/19/25 3:47 PM, Thomas Zimmermann wrote:
>>>>>> Hi
>>>>>>
>>>>>> Am 19.10.25 um 16:34 schrieb Greg KH:
>>>>>>> On Sun, Oct 19, 2025 at 04:12:28PM +0100, Mehdi Ben Hadj Khelifa 
>>>>>>> wrote:
>>>>>>>> Replace kmalloc() with kmalloc_array() to correctly
>>>>>>>> handle array allocations and benefit from built-in overflow 
>>>>>>>> checking[1].
>>>>>>>>
>>>>>>>> [1]:https://docs.kernel.org/process/deprecated.html
>>>>>>>>
>>>>>>>> Signed-off-by: Mehdi Ben Hadj Khelifa 
>>>>>>>> <mehdi.benhadjkhelifa@gmail.com>
>>>>>>>> ---
>>>>>>>>    drivers/gpu/drm/tiny/repaper.c | 2 +-
>>>>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/
>>>>>>>> repaper.c
>>>>>>>> index 4824f863fdba..290132c24ff9 100644
>>>>>>>> --- a/drivers/gpu/drm/tiny/repaper.c
>>>>>>>> +++ b/drivers/gpu/drm/tiny/repaper.c
>>>>>>>> @@ -534,7 +534,7 @@ static int repaper_fb_dirty(struct
>>>>>>>> drm_framebuffer *fb, const struct iosys_map *
>>>>>>>>        DRM_DEBUG("Flushing [FB:%d] st=%ums\n", fb->base.id,
>>>>>>>>              epd->factored_stage_time);
>>>>>>>> -    buf = kmalloc(fb->width * fb->height / 8, GFP_KERNEL);
>>>>>>>> +    buf = kmalloc_array(fb->height / 8, fb->width, GFP_KERNEL);
>>>>
>>>> Also worth emphasizing that this is wildly wrong for any height that is
>>>> not a multiple of 8.
>>>>
>>>> And I thought I shot down a similar patch not long ago.
>>>>
>>>> Is there some tool that suggests doing this? Fix the tool instead
>>>> please.
>>>>
>>>
>>> They are documented in 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.
>>>
>> I have sent an appropriate v2 specifically to suit the case that we 
>> have here. But 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? Next time give more details on the
> where you found the problem - it is easy to miss the link unless you
> state that it is coming from the deprecated document.
> 
For my testing I have used a raspberry pi zero 2wh with an e-paper 
display to test. I have installed the custom kernel with my 
patch.Confirmed module loading in dmesg and ran modetest with no signs 
of regressions or errors in dmesg.
If further proof is needed I will be happy to provide it.
I will be mentionning the deprecated document more clearly next time.
> Even so you have to explain why the change is applicable to the code
> you are changing. How are you testing these changes. I have seen more
> patches from you in drm drivers and lib code.
> 
I have sent a v2 and it has the more suitable changes for this case as i 
said which was suggested by thomas. here is v2.
Important to note that my last testing was on v2 changes.
Link:https://lore.kernel.org/all/20251020115803.192572-1-mehdi.benhadjkhelifa@gmail.com/> 
thanks,
> -- Shuah

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

Posted by Thomas Zimmermann 1 month, 4 weeks ago

Hi

Am 20.10.25 um 11:50 schrieb Jani Nikula:
> On Sun, 19 Oct 2025, Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com> wrote:
>> On 10/19/25 3:47 PM, Thomas Zimmermann wrote:
>>> Hi
>>>
>>> Am 19.10.25 um 16:34 schrieb Greg KH:
>>>> On Sun, Oct 19, 2025 at 04:12:28PM +0100, Mehdi Ben Hadj Khelifa wrote:
>>>>> Replace kmalloc() with kmalloc_array() to correctly
>>>>> handle array allocations and benefit from built-in overflow checking[1].
>>>>>
>>>>> [1]:https://docs.kernel.org/process/deprecated.html
>>>>>
>>>>> Signed-off-by: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@gmail.com>
>>>>> ---
>>>>>    drivers/gpu/drm/tiny/repaper.c | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/tiny/repaper.c b/drivers/gpu/drm/tiny/
>>>>> repaper.c
>>>>> index 4824f863fdba..290132c24ff9 100644
>>>>> --- a/drivers/gpu/drm/tiny/repaper.c
>>>>> +++ b/drivers/gpu/drm/tiny/repaper.c
>>>>> @@ -534,7 +534,7 @@ static int repaper_fb_dirty(struct
>>>>> drm_framebuffer *fb, const struct iosys_map *
>>>>>        DRM_DEBUG("Flushing [FB:%d] st=%ums\n", fb->base.id,
>>>>>              epd->factored_stage_time);
>>>>> -    buf = kmalloc(fb->width * fb->height / 8, GFP_KERNEL);
>>>>> +    buf = kmalloc_array(fb->height / 8, fb->width, GFP_KERNEL);
> Also worth emphasizing that this is wildly wrong for any height that is
> not a multiple of 8.

Yes. I skipped over details as the format helpers would solve this 
problem if done correctly.

>
> And I thought I shot down a similar patch not long ago.

You did AFAIR.

>
> Is there some tool that suggests doing this? Fix the tool instead
> please.

There's this todo item that the patch refers to. Volunteers discover 
these and start working. But without mentoring, it's often not to 
anyone's benefit. I've noticed a similar pattern wrt the DRM todo list.

Best regards
Thomas

>
> BR,
> Jani.
>
>
>
>
>>>> This isn't an array, so this function change doesn't seem to make much
>>>> sense, right?  The size should have already been checked earlier in the
>>>> call change to be correct.
>> Yes,I was intending to say framebuffer but I was working on another
>> similar patch simultaneously so I reused same words by mistake. Thanks
>> for clarifying that.>
>>> Yes, we've recently received plenty of these pointless changes. The
>>> correct code would compute the number of bytes per pixel using
>>> drm_format_info_min_pitch() and multiply with fb->height. The latter
>>> could (maybe) use kmalloc_array(). It would still not be an array in the
>>> common sense.
>>>
>> Thanks for the review and suggestion.I will be sending a v2 patch with
>> the recommended code change.
>>
>> Best Regards,
>> Mehdi Ben Hadj Khelifa> Best regards
>>> Thomas
>>>
>>>> thanks,
>>>>
>>>> greg k-h
>>> -- 
>>> -- 
>>> Thomas Zimmermann
>>> Graphics Driver Developer
>>> SUSE Software Solutions Germany GmbH
>>> Frankenstrasse 146, 90461 Nuernberg, Germany
>>> GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
>>> HRB 36809 (AG Nuernberg)

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)