[PATCH] drm/amdgpu/atom: do not dereference NULL vbios string pointer

[PATCH] drm/amdgpu/atom: do not dereference NULL vbios string pointer

[Noah Weinberger]

Hello,

In atom_get_vbios_pn(), when the part-number string is empty and
atom_find_str_in_rom() cannot find the ATOMBIOS anchor, the fallback
computes NULL + sizeof(BIOS_ATOM_PREFIX) - 1, i.e. (unsigned char *)8:

    vbios_str = atom_find_str_in_rom(ctx, BIOS_ATOM_PREFIX, 3, 1024, 64);
    if (vbios_str == NULL)
            vbios_str += sizeof(BIOS_ATOM_PREFIX) - 1;

The subsequent range check is false for a near-NULL pointer, so the
value survives and *vbios_str dereferences address 8 - a NULL-page oops
during probe with a malformed VBIOS that passes the magic checks. The
recent hardening of the neighboring date/version/build helpers closed
their OOB reads; this fallback path was left intact.

Simply leave the part number empty when the anchor is not found.

Affected version/commit: mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07), drivers/gpu/drm/amd/amdgpu/atom.c:1491-1503 (atom_get_vbios_pn).
Pointer-arithmetic-on-NULL is pre-existing; this window's hardening series made it the sole fallback outcome.

This was found during an assisted (AI-supported) static source review of
mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07); every code path cited below was re-verified by the
reporter against that tree. The patch below has NOT been compiled and has
NOT been runtime tested, and no reproducer was produced - the triggering
conditions are described above instead. Please treat this as a report for
verification rather than a battle-tested fix.

Reported-by: Noah Weinberger <noahweinberger2003@proton.me>
---
diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
index 1a599d5..7598606 100644
--- a/drivers/gpu/drm/amd/amdgpu/atom.c
+++ b/drivers/gpu/drm/amd/amdgpu/atom.c
@@ -1493,8 +1493,6 @@ static void atom_get_vbios_pn(struct atom_context *ctx)
 
 	if (vbios_str == NULL || *vbios_str == 0) {
 		vbios_str = atom_find_str_in_rom(ctx, BIOS_ATOM_PREFIX, 3, 1024, 64);
-		if (vbios_str == NULL)
-			vbios_str += sizeof(BIOS_ATOM_PREFIX) - 1;
 		if (vbios_str >= p_rom + ctx->bios_size)
 			vbios_str = NULL;
 	}

Re: [PATCH] drm/amdgpu/atom: do not dereference NULL vbios string pointer

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] drm/amdgpu/atom: broken BIOS_ATOM_PREFIX fallback in
  atom_get_vbios_pn()
- [Low] drm/amdgpu/atom: missing Fixes and Signed-off-by trailers in
  commit log
It also noticed 1 related pre-existing issue this patch does not
introduce:
- [Critical] drm/amdgpu: buffer underflow in atom_get_vbios_name() [1]
--

commit d8195d5b6067da759e75cbe7469c7a841f107f37
Author: Noah Weinberger <NoahWeinberger2003@proton.me>

> drm/amdgpu/atom: do not dereference NULL vbios string pointer
>
> Hello,
>
> In atom_get_vbios_pn(), when the part-number string is empty and
> atom_find_str_in_rom() cannot find the ATOMBIOS anchor, the fallback
> computes NULL + sizeof(BIOS_ATOM_PREFIX) - 1, i.e. (unsigned char *)8:
>
>     vbios_str = atom_find_str_in_rom(ctx, BIOS_ATOM_PREFIX, 3, 1024, 64);
>     if (vbios_str == NULL)
>             vbios_str += sizeof(BIOS_ATOM_PREFIX) - 1;
>
> The subsequent range check is false for a near-NULL pointer, so the
> value survives and *vbios_str dereferences address 8 - a NULL-page oops
> during probe with a malformed VBIOS that passes the magic checks. The
> recent hardening of the neighboring date/version/build helpers closed
> their OOB reads; this fallback path was left intact.
>
> Simply leave the part number empty when the anchor is not found.
>
> Affected version/commit: mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07), drivers/gpu/drm/amd/amdgpu/atom.c:1491-1503 (atom_get_vbios_pn).
> Pointer-arithmetic-on-NULL is pre-existing; this window's hardening series made it the sole fallback outcome.
>
> This was found during an assisted (AI-supported) static source review of
> mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07); every code path cited below was re-verified by the
> reporter against that tree. The patch below has NOT been compiled and has
> NOT been runtime tested, and no reproducer was produced - the triggering
>conditions are described above instead. Please treat this as a report for
> verification rather than a battle-tested fix.
>
> Reported-by: Noah Weinberger <noahweinberger2003@proton.me>

[Severity: Low]
This commit is missing a Signed-off-by trailer from the author, and the
greeting and report disclaimers belong below the --- separator rather than
in the permanent changelog.  Should it also include a Fixes tag for the
commit that introduced the inverted check:

  Fixes: 29b4c589b43d ("drm/amdgpu: Add vbios info ioctl interface")

> diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
> index 1a599d5cf6a5c..7598606afaef1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/atom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/atom.c
> @@ -1493,8 +1493,6 @@ static void atom_get_vbios_pn(struct atom_context *ctx)
>  
>  	if (vbios_str == NULL || *vbios_str == 0) {
>  		vbios_str = atom_find_str_in_rom(ctx, BIOS_ATOM_PREFIX, 3, 1024, 64);
> -		if (vbios_str == NULL)
> -			vbios_str += sizeof(BIOS_ATOM_PREFIX) - 1;
>  		if (vbios_str >= p_rom + ctx->bios_size)
>  			vbios_str = NULL;
>  	}

[Severity: Medium]
Should this check be inverted to "if (vbios_str != NULL)" instead of
removing the pointer advance?

atom_find_str_in_rom() returns a pointer to the start of the matched
"ATOMBIOS" prefix, which in the ROM layout is followed by a NUL byte and
the part-number string.  Removing the advance leaves vbios_str at the
prefix itself, so the subsequent NUL-skip check:

drivers/gpu/drm/amd/amdgpu/atom.c:atom_get_vbios_pn() {
	...
	OPTIMIZER_HIDE_VAR(vbios_str);
	if (vbios_str != NULL && *vbios_str == 0)
		vbios_str++;
}

becomes dead code and the copy loop populates ctx->vbios_pn with the
literal string "ATOMBIOS" instead of the part number.  Once the NULL
pointer arithmetic is gone, OPTIMIZER_HIDE_VAR(vbios_str) from
e5f7e4e0a445 ("drm/amdgpu/atom: Work around vbios NULL offset false
positive") can also be dropped.

[Severity: Critical]
This problem wasn't introduced by this patch, but can the space-trimming
loop in the sibling helper atom_get_vbios_name() underflow ctx->name[]
[1]?

drivers/gpu/drm/amd/amdgpu/atom.c:atom_get_vbios_name() {
	...
	name_size = strnlen(c_ptr, min(STRLEN_LONG - 1, (int)(p_end - c_ptr)));
	memcpy(ctx->name, c_ptr, name_size);
	back = ctx->name + name_size;
	while ((*--back) == ' ')
		;
	*(back + 1) = '\0';
	...
}

If the VBIOS name string consists entirely of spaces (or is empty and the
preceding byte in struct atom_context happens to be 0x20), back decrements
past the start of ctx->name[] until it hits a non-space byte and then
overwrites the byte after it with a NUL terminator, corrupting earlier
fields of struct atom_context.

[1] https://sashiko.dev/#/bug/linux-ad8886ee-341a-464b-9d34-3723f5f4a62e

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/4VYoOFUjYIK--SKrUsijHVOOHA6ruEc8z_hXR_OMj7syXD6492lPaEKmsRRV3iLW1QAkn2X29F7_-lVKnIx4vUE1KQtKx8Zdk8sX_rFaZIA=@proton.me?part=1

Re: [PATCH] drm/amdgpu/atom: do not dereference NULL vbios string pointer

[Alex Deucher]

On Mon, Nov 9, 2026 at 4:41 AM Noah Weinberger
<NoahWeinberger2003@proton.me> wrote:
>
> Hello,
>
> In atom_get_vbios_pn(), when the part-number string is empty and
> atom_find_str_in_rom() cannot find the ATOMBIOS anchor, the fallback
> computes NULL + sizeof(BIOS_ATOM_PREFIX) - 1, i.e. (unsigned char *)8:
>
>     vbios_str = atom_find_str_in_rom(ctx, BIOS_ATOM_PREFIX, 3, 1024, 64);
>     if (vbios_str == NULL)
>             vbios_str += sizeof(BIOS_ATOM_PREFIX) - 1;
>
> The subsequent range check is false for a near-NULL pointer, so the
> value survives and *vbios_str dereferences address 8 - a NULL-page oops
> during probe with a malformed VBIOS that passes the magic checks. The
> recent hardening of the neighboring date/version/build helpers closed
> their OOB reads; this fallback path was left intact.
>
> Simply leave the part number empty when the anchor is not found.
>
> Affected version/commit: mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07), drivers/gpu/drm/amd/amdgpu/atom.c:1491-1503 (atom_get_vbios_pn).
> Pointer-arithmetic-on-NULL is pre-existing; this window's hardening series made it the sole fallback outcome.
>
> This was found during an assisted (AI-supported) static source review of
> mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07); every code path cited below was re-verified by the
> reporter against that tree. The patch below has NOT been compiled and has
> NOT been runtime tested, and no reproducer was produced - the triggering
> conditions are described above instead. Please treat this as a report for
> verification rather than a battle-tested fix.
>
> Reported-by: Noah Weinberger <noahweinberger2003@proton.me>

Missing your Signed-off-by.

Alex

> ---
> diff --git a/drivers/gpu/drm/amd/amdgpu/atom.c b/drivers/gpu/drm/amd/amdgpu/atom.c
> index 1a599d5..7598606 100644
> --- a/drivers/gpu/drm/amd/amdgpu/atom.c
> +++ b/drivers/gpu/drm/amd/amdgpu/atom.c
> @@ -1493,8 +1493,6 @@ static void atom_get_vbios_pn(struct atom_context *ctx)
>
>         if (vbios_str == NULL || *vbios_str == 0) {
>                 vbios_str = atom_find_str_in_rom(ctx, BIOS_ATOM_PREFIX, 3, 1024, 64);
> -               if (vbios_str == NULL)
> -                       vbios_str += sizeof(BIOS_ATOM_PREFIX) - 1;
>                 if (vbios_str >= p_rom + ctx->bios_size)
>                         vbios_str = NULL;
>         }
>

Re: [PATCH] drm/amdgpu/atom: do not dereference NULL vbios string pointer

[Krzysztof Kozlowski]

On 11/11/2026 17:57, Alex Deucher wrote:
> On Mon, Nov 9, 2026 at 4:41 AM Noah Weinberger
> <NoahWeinberger2003@proton.me> wrote:
>>
>> Hello,
>>
>> In atom_get_vbios_pn(), when the part-number string is empty and
>> atom_find_str_in_rom() cannot find the ATOMBIOS anchor, the fallback
>> computes NULL + sizeof(BIOS_ATOM_PREFIX) - 1, i.e. (unsigned char *)8:
>>
>>     vbios_str = atom_find_str_in_rom(ctx, BIOS_ATOM_PREFIX, 3, 1024, 64);
>>     if (vbios_str == NULL)
>>             vbios_str += sizeof(BIOS_ATOM_PREFIX) - 1;
>>
>> The subsequent range check is false for a near-NULL pointer, so the
>> value survives and *vbios_str dereferences address 8 - a NULL-page oops
>> during probe with a malformed VBIOS that passes the magic checks. The
>> recent hardening of the neighboring date/version/build helpers closed
>> their OOB reads; this fallback path was left intact.
>>
>> Simply leave the part number empty when the anchor is not found.
>>
>> Affected version/commit: mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07), drivers/gpu/drm/amd/amdgpu/atom.c:1491-1503 (atom_get_vbios_pn).
>> Pointer-arithmetic-on-NULL is pre-existing; this window's hardening series made it the sole fallback outcome.
>>
>> This was found during an assisted (AI-supported) static source review of
>> mainline fa4ad798f6f681031775266044ba1b0cf3c670bd (v7.4-rc1, 2026-11-07); every code path cited below was re-verified by the
>> reporter against that tree. The patch below has NOT been compiled and has
>> NOT been runtime tested, and no reproducer was produced - the triggering
>> conditions are described above instead. Please treat this as a report for
>> verification rather than a battle-tested fix.
>>
>> Reported-by: Noah Weinberger <noahweinberger2003@proton.me>
> 
> Missing your Signed-off-by.

That is not a patch, but a report. It wasn't tested, wasn't even
verified and is possible that it is LLM slop like the rest.

Best regards,
Krzysztof