Re: [PATCH] drm/amdgpu/atom: Work around vbios NULL offset false positive

[Kees Cook <kees@kernel.org>]

On Tue, Apr 22, 2025 at 09:22:26AM -0400, Alex Deucher wrote:
> On Mon, Apr 21, 2025 at 5:59 PM Kees Cook <kees@kernel.org> wrote:
> >
> > GCC really does not want to consider NULL (or near-NULL) addresses as
> > valid, so calculations based off of NULL end up getting range-tracked into
> > being an offset wthin a 0 byte array. It gets especially mad about this:
> >
> >                 if (vbios_str == NULL)
> >                         vbios_str += sizeof(BIOS_ATOM_PREFIX) - 1;
> >         ...
> >         if (vbios_str != NULL && *vbios_str == 0)
> >                 vbios_str++;
> >
> > It sees this as being "sizeof(BIOS_ATOM_PREFIX) - 1" byte offset from
> > NULL, when building with -Warray-bounds (and the coming
> > -fdiagnostic-details flag):
> >
> > In function 'atom_get_vbios_pn',
> >     inlined from 'amdgpu_atom_parse' at drivers/gpu/drm/amd/amdgpu/atom.c:1553:2:
> > drivers/gpu/drm/amd/amdgpu/atom.c:1447:34: error: array subscript 0 is outside array bounds of 'unsigned char[0]' [-Werror=array-bounds=]
> >  1447 |         if (vbios_str != NULL && *vbios_str == 0)
> >       |                                  ^~~~~~~~~~
> >   'amdgpu_atom_parse': events 1-2
> >  1444 |                 if (vbios_str == NULL)
> >       |                    ^
> >       |                    |
> >       |                    (1) when the condition is evaluated to true
> > ......
> >  1447 |         if (vbios_str != NULL && *vbios_str == 0)
> >       |                                  ~~~~~~~~~~
> >       |                                  |
> >       |                                  (2) out of array bounds here
> > In function 'amdgpu_atom_parse':
> > cc1: note: source object is likely at address zero
> >
> > As there isn't a sane way to convince it otherwise, hide vbios_str from
> > GCC's optimizer to avoid the warning so we can get closer to enabling
> > -Warray-bounds globally.
> >
> > Signed-off-by: Kees Cook <kees@kernel.org>
> 
> Acked-by: Alex Deucher <alexander.deucher@amd.com>

Thanks!

> Do you want me to pick this up, or do you want to take this through
> some other tree?

Whatever is easier for you. I'm happy to carry it if you'd like. :)
There's no rush on these -- it's been a long road to getting
-Warray-bounds enabled. ;)

-- 
Kees Cook