[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b0b1c2e9-c367-4e9c-b931-d3e1b0ba7f5b@gmail.com>
Date: Mon, 20 Oct 2025 22:11:24 +0100
From: Mehdi Ben Hadj Khelifa <mehdi.benhadjkhelifa@...il.com>
To: Shuah Khan <skhan@...uxfoundation.org>,
Jani Nikula <jani.nikula@...ux.intel.com>,
Thomas Zimmermann <tzimmermann@...e.de>, Greg KH <gregkh@...uxfoundation.org>
Cc: lanzano.alex@...il.com, maarten.lankhorst@...ux.intel.com,
mripard@...nel.org, airlied@...il.com, simona@...ll.ch,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
david.hunter.linux@...il.com, khalid@...nel.org,
linux-kernel-mentees@...ts.linuxfoundation.org
Subject: Re: [PATCH] drm/tiny: Use kmalloc_array() instead of kmalloc()
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@...il.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@...il.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
Powered by blists - more mailing lists