linux-kernel - Re: [PATCH] drm/radeon/r100: enhance error handling in r100_cp_init

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAABZP2zGfhcYrn6UOhmkXEgtCAPLcN7bgCdbzAi1r+aNrToHKQ@mail.gmail.com>
Date: Tue, 28 May 2024 09:36:58 +0800
From: Zhouyi Zhou <zhouzhouyi@...il.com>
To: Christian König <christian.koenig@....com>
Cc: alexander.deucher@....com, Xinhui.Pan@....com, airlied@...il.com, 
	daniel@...ll.ch, chris@...isdown.name, amd-gfx@...ts.freedesktop.org, 
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, 
	rcu@...r.kernel.org, lance@...osl.org
Subject: Re: [PATCH] drm/radeon/r100: enhance error handling in r100_cp_init_microcode

Thanks for reviewing the patch

On Mon, May 27, 2024 at 3:58 PM Christian König
<christian.koenig@....com> wrote:
>
> Am 27.05.24 um 03:20 schrieb Zhouyi Zhou:
> > In r100_cp_init_microcode, if rdev->family don't match any of
> > if statement,  fw_name will be NULL, which will cause
> > gcc (11.4.0 powerpc64le-linux-gnu) complain:
> >
> > In function ‘r100_cp_init_microcode’,
> >      inlined from ‘r100_cp_init’ at drivers/gpu/drm/radeon/r100.c:1136:7:
> > ./include/linux/printk.h:457:44: warning: ‘%s’ directive argument is null [-Wformat-overflow=]
> >    457 | #define printk(fmt, ...) printk_index_wrap(_printk, fmt, ##__VA_ARGS__)
> >
> > Above warning is emitted during the rcutorture test in
> > in PPC VM of Opensource Lab of Oregon State Univerisity.
> >
> > Enhance error handling in r100_cp_init_microcode, let r100_cp_init_microcode
> > return with -EINVAL when none of chip families is matched.
> >
> > Signed-off-by: Zhouyi Zhou <zhouzhouyi@...il.com>
>
> > ---
> >   drivers/gpu/drm/radeon/r100.c | 7 +++++++
> >   1 file changed, 7 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/radeon/r100.c b/drivers/gpu/drm/radeon/r100.c
> > index 0b1e19345f43..4f8a1bdd9365 100644
> > --- a/drivers/gpu/drm/radeon/r100.c
> > +++ b/drivers/gpu/drm/radeon/r100.c
> > @@ -1055,6 +1055,11 @@ static int r100_cp_init_microcode(struct radeon_device *rdev)
> >                  (rdev->family == CHIP_RV570)) {
> >               DRM_INFO("Loading R500 Microcode\n");
> >               fw_name = FIRMWARE_R520;
> > +     } else {
> > +             pr_err("radeon_cp: Failed to load firmware \"%d\"\n",
> > +                     rdev->family);
> > +             err = -EINVAL;
> > +             goto out;
> >       }
> >
> >       err = request_firmware(&rdev->me_fw, fw_name, rdev->dev);
> > @@ -1067,6 +1072,8 @@ static int r100_cp_init_microcode(struct radeon_device *rdev)
> >               release_firmware(rdev->me_fw);
> >               rdev->me_fw = NULL;
> >       }
> > +
> > +out:
>
> That looks superfluous, just return -EINVAL directly in the else case above.
>
> Apart from that this is for ~15year old hardware. I'm a bit reluctant
> adding code for something that old even when this change here looks
> harmless.
>
> Is there a plan to complain about that in an automated checker? If yes
> then the change is probably justified, if no then I would rather not do it.
The warning is emitted when I invoke following commands in ubuntu
22.04 (ppc64le)
linux$make allmodconfig
linux$make drivers/gpu/drm/radeon/r100.o (a quick alternative to 'make -j$nproc)

But everything is OK when I invoke following commands in ubuntu 22.04 (ppc64le)
linux$make allmodconfig LLVM=1
linux$make drivers/gpu/drm/radeon/r100.o LLVM=1

And I can't reproduce the warning in the x86 environment, so I guess
this phenomenon
is toolchain related ;-)

Thanks again
Regards,
Zhouyi
>
> Regards,
> Christian.
>
> >       return err;
> >   }
> >
>