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

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: <CADnq5_MbGS+DBRZhQccqP-o50vvv6uiT31msefRTw5bMydAAKg@mail.gmail.com>
Date: Tue, 22 Apr 2025 09:22:26 -0400
From: Alex Deucher <alexdeucher@...il.com>
To: Kees Cook <kees@...nel.org>
Cc: Alex Deucher <alexander.deucher@....com>, 
	Christian König <christian.koenig@....com>, 
	Xinhui Pan <Xinhui.Pan@....com>, David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, 
	Jesse Zhang <jesse.zhang@....com>, Tim Huang <Tim.Huang@....com>, 
	Srinivasan Shanmugam <srinivasan.shanmugam@....com>, Alexander Richards <electrodeyt@...il.com>, 
	Lijo Lazar <lijo.lazar@....com>, Mario Limonciello <mario.limonciello@....com>, 
	"Gustavo A. R. Silva" <gustavoars@...nel.org>, amd-gfx@...ts.freedesktop.org, 
	dri-devel@...ts.freedesktop.org, Al Viro <viro@...iv.linux.org.uk>, 
	linux-kernel@...r.kernel.org, linux-hardening@...r.kernel.org
Subject: Re: [PATCH] drm/amdgpu/atom: Work around vbios NULL offset false positive

On Mon, Apr 21, 2025 at 5:59 PM Kees Cook <kees@...nel.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@...nel.org>

Acked-by: Alex Deucher <alexander.deucher@....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@....com>
> Cc: "Christian König" <christian.koenig@....com>
> Cc: Xinhui Pan <Xinhui.Pan@....com>
> Cc: David Airlie <airlied@...il.com>
> Cc: Simona Vetter <simona@...ll.ch>
> Cc: Jesse Zhang <jesse.zhang@....com>
> Cc: Tim Huang <Tim.Huang@....com>
> Cc: Srinivasan Shanmugam <srinivasan.shanmugam@....com>
> Cc: Alexander Richards <electrodeyt@...il.com>
> Cc: Lijo Lazar <lijo.lazar@....com>
> Cc: Mario Limonciello <mario.limonciello@....com>
> Cc: "Gustavo A. R. Silva" <gustavoars@...nel.org>
> Cc: amd-gfx@...ts.freedesktop.org
> Cc: dri-devel@...ts.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
>