Re: [PATCH] video: fbdev: arkfb: Cast ics5342_init() allocation type

[Helge Deller <deller@gmx.de>]

On 4/28/25 08:36, Geert Uytterhoeven wrote:
> Hi Kees,
> 
> On Sat, 26 Apr 2025 at 13:33, Helge Deller <deller@gmx.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@kernel.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.

Helge