* [PATCH] drm/amdgpu/atom: Work around vbios NULL offset false positive
@ 2025-04-21 21:58 Kees Cook
2025-04-22 13:22 ` Alex Deucher
0 siblings, 1 reply; 4+ messages in thread
From: Kees Cook @ 2025-04-21 21:58 UTC (permalink / raw)
To: Alex Deucher
Cc: Kees Cook, Christian König, Xinhui Pan, David Airlie,
Simona Vetter, Jesse Zhang, Tim Huang, Srinivasan Shanmugam,
Alexander Richards, Lijo Lazar, Mario Limonciello,
Gustavo A. R. Silva, amd-gfx, dri-devel, Al Viro, linux-kernel,
linux-hardening
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>
---
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: Xinhui Pan <Xinhui.Pan@amd.com>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: Jesse Zhang <jesse.zhang@amd.com>
Cc: Tim Huang <Tim.Huang@amd.com>
Cc: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
Cc: Alexander Richards <electrodeyt@gmail.com>
Cc: Lijo Lazar <lijo.lazar@amd.com>
Cc: Mario Limonciello <mario.limonciello@amd.com>
Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
Cc: amd-gfx@lists.freedesktop.org
Cc: dri-devel@lists.freedesktop.org
---
drivers/gpu/drm/amd/amdgpu/atom.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
index 81d195d366ce..427b073de2fc 100644
--- a/drivers/gpu/drm/amd/amdgpu/atom.c
+++ b/drivers/gpu/drm/amd/amdgpu/atom.c
@@ -1444,6 +1444,7 @@ static void atom_get_vbios_pn(struct atom_context *ctx)
if (vbios_str == NULL)
vbios_str += sizeof(BIOS_ATOM_PREFIX) - 1;
}
+ OPTIMIZER_HIDE_VAR(vbios_str);
if (vbios_str != NULL && *vbios_str == 0)
vbios_str++;
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] drm/amdgpu/atom: Work around vbios NULL offset false positive
2025-04-21 21:58 [PATCH] drm/amdgpu/atom: Work around vbios NULL offset false positive Kees Cook
@ 2025-04-22 13:22 ` Alex Deucher
2025-04-22 14:56 ` Kees Cook
0 siblings, 1 reply; 4+ messages in thread
From: Alex Deucher @ 2025-04-22 13:22 UTC (permalink / raw)
To: Kees Cook
Cc: Alex Deucher, Christian König, Xinhui Pan, David Airlie,
Simona Vetter, Jesse Zhang, Tim Huang, Srinivasan Shanmugam,
Alexander Richards, Lijo Lazar, Mario Limonciello,
Gustavo A. R. Silva, amd-gfx, dri-devel, Al Viro, linux-kernel,
linux-hardening
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>
Do you want me to pick this up, or do you want to take this through
some other tree?
Thanks,
Alex
> ---
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: Xinhui Pan <Xinhui.Pan@amd.com>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: Jesse Zhang <jesse.zhang@amd.com>
> Cc: Tim Huang <Tim.Huang@amd.com>
> Cc: Srinivasan Shanmugam <srinivasan.shanmugam@amd.com>
> Cc: Alexander Richards <electrodeyt@gmail.com>
> Cc: Lijo Lazar <lijo.lazar@amd.com>
> Cc: Mario Limonciello <mario.limonciello@amd.com>
> Cc: "Gustavo A. R. Silva" <gustavoars@kernel.org>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: dri-devel@lists.freedesktop.org
> ---
> drivers/gpu/drm/amd/amdgpu/atom.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
> index 81d195d366ce..427b073de2fc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/atom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/atom.c
> @@ -1444,6 +1444,7 @@ static void atom_get_vbios_pn(struct atom_context *ctx)
> if (vbios_str == NULL)
> vbios_str += sizeof(BIOS_ATOM_PREFIX) - 1;
> }
> + OPTIMIZER_HIDE_VAR(vbios_str);
> if (vbios_str != NULL && *vbios_str == 0)
> vbios_str++;
>
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/amdgpu/atom: Work around vbios NULL offset false positive
2025-04-22 13:22 ` Alex Deucher
@ 2025-04-22 14:56 ` Kees Cook
2025-04-22 15:24 ` Alex Deucher
0 siblings, 1 reply; 4+ messages in thread
From: Kees Cook @ 2025-04-22 14:56 UTC (permalink / raw)
To: Alex Deucher
Cc: Alex Deucher, Christian König, Xinhui Pan, David Airlie,
Simona Vetter, Jesse Zhang, Tim Huang, Srinivasan Shanmugam,
Alexander Richards, Lijo Lazar, Mario Limonciello,
Gustavo A. R. Silva, amd-gfx, dri-devel, Al Viro, linux-kernel,
linux-hardening
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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/amdgpu/atom: Work around vbios NULL offset false positive
2025-04-22 14:56 ` Kees Cook
@ 2025-04-22 15:24 ` Alex Deucher
0 siblings, 0 replies; 4+ messages in thread
From: Alex Deucher @ 2025-04-22 15:24 UTC (permalink / raw)
To: Kees Cook
Cc: Alex Deucher, Christian König, Xinhui Pan, David Airlie,
Simona Vetter, Jesse Zhang, Tim Huang, Srinivasan Shanmugam,
Alexander Richards, Lijo Lazar, Mario Limonciello,
Gustavo A. R. Silva, amd-gfx, dri-devel, Al Viro, linux-kernel,
linux-hardening
On Tue, Apr 22, 2025 at 10:56 AM Kees Cook <kees@kernel.org> wrote:
>
> 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. ;)
I've picked it up. Thanks!
Alex
>
> --
> Kees Cook
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-04-22 15:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-21 21:58 [PATCH] drm/amdgpu/atom: Work around vbios NULL offset false positive Kees Cook
2025-04-22 13:22 ` Alex Deucher
2025-04-22 14:56 ` Kees Cook
2025-04-22 15:24 ` Alex Deucher
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.