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

Mehdi Ben Hadj Khelifa
2025-10-20T21:00:41Z

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