linux-kernel - Re: [PATCH] video: fbdev: arkfb: Cast ics5342

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAMuHMdVY1_gEqULGD0BzdTd05OAkodhk+RXKRAy-T-0+RJt7yQ@mail.gmail.com>
Date: Mon, 28 Apr 2025 08:36:40 +0200
From: Geert Uytterhoeven <geert@...ux-m68k.org>
To: Kees Cook <kees@...nel.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, Helge Deller <deller@....de>
Subject: Re: [PATCH] video: fbdev: arkfb: Cast ics5342_init() allocation type

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.

> > --- a/drivers/video/fbdev/arkfb.c
> > +++ b/drivers/video/fbdev/arkfb.c
> > @@ -431,7 +431,7 @@ 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 = kzalloc(sizeof(struct ics5342_info), GFP_KERNEL);
> > +     struct dac_info *info = (struct dac_info *)kzalloc(sizeof(struct ics5342_info), GFP_KERNEL);
> >
> >       if (! info)
> >               return NULL;

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds