[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <002CF88F-6023-4A1F-A436-EE3720BD7B7B@kernel.org>
Date: Tue, 29 Apr 2025 13:25:55 -0700
From: Kees Cook <kees@...nel.org>
To: Helge Deller <deller@....de>, Geert Uytterhoeven <geert@...ux-m68k.org>
CC: Javier Martinez Canillas <javierm@...hat.com>,
Thomas Zimmermann <tzimmermann@...e.de>, Zheyu Ma <zheyuma97@...il.com>,
Samuel Thibault <samuel.thibault@...-lyon.org>,
Jiapeng Chong <jiapeng.chong@...ux.alibaba.com>, linux-fbdev@...r.kernel.org,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
linux-hardening@...r.kernel.org
Subject: Re: [PATCH] video: fbdev: arkfb: Cast ics5342_init() allocation type
On April 29, 2025 1:17:26 PM PDT, Helge Deller <deller@....de> wrote:
>On 4/28/25 08:36, Geert Uytterhoeven wrote:
>> Hi Kees,
>>
>> On Sat, 26 Apr 2025 at 13:33, Helge Deller <deller@....de> wrote:
>>> On 4/26/25 08:23, Kees Cook wrote:
>>>> In preparation for making the kmalloc family of allocators type aware,
>>>> we need to make sure that the returned type from the allocation matches
>>>> the type of the variable being assigned. (Before, the allocator would
>>>> always return "void *", which can be implicitly cast to any pointer type.)
>>>>
>>>> The assigned type is "struct dac_info *" but the returned type will be
>>>> "struct ics5342_info *", which has a larger allocation size. This is
>>>> by design, as struct ics5342_info contains struct dac_info as its first
>>>> member. Cast the allocation type to match the assignment.
>>>>
>>>> Signed-off-by: Kees Cook <kees@...nel.org>
>>
>> Thanks for your patch, which is now commit 8d2f0f5bbac87b9d ("fbdev:
>> arkfb: Cast ics5342_init() allocation type") in fbdev/for-next.
>>
>>> I applied your patch, but wouldn't this untested patch be cleaner and fulfill the
>>> same purpose to match a kzalloc return type?
>>>
>>> diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
>>> index 7d131e3d159a..a57c8a992e11 100644
>>> --- a/drivers/video/fbdev/arkfb.c
>>> +++ b/drivers/video/fbdev/arkfb.c
>>> @@ -431,7 +431,8 @@ static struct dac_ops ics5342_ops = {
>>>
>>> static struct dac_info * ics5342_init(dac_read_regs_t drr, dac_write_regs_t dwr, void *data)
>>> {
>>> - struct dac_info *info = (struct dac_info *)kzalloc(sizeof(struct ics5342_info), GFP_KERNEL);
>>> + struct ics5342_info *ics_info = kzalloc(sizeof(struct ics5342_info), GFP_KERNEL);
>>
>> sizeof(*ics_info)?
>>
>>> + struct dac_info *info = &ics_info->dac;
>>
>> Exactly my thought when I noticed this commit. Adding casts makes
>> it harder to notice any future discrepancies.
>
>I've changed it accordingly.
Thanks! Yeah, that's a much nicer solution.
--
Kees Cook
Powered by blists - more mailing lists