[PATCH] drm/amdgpu/gfx8: fix register_list_format leak in gfx_v8_0_init_microcode

[PATCH] drm/amdgpu/gfx8: fix register_list_format leak in gfx_v8_0_init_microcode

[Willian Oliveira]

gfx_v8_0_init_microcode() allocates rlc.register_list_format before
loading the MEC firmware.

If loading one of the subsequent firmware files fails, the function
returns through the error path without freeing register_list_format.

Use gfx_v8_0_free_microcode() in the error path so all previously
allocated firmware resources, including register_list_format, are
released.

Signed-off-by: Willian Oliveira <williandossantosdeoliveira287@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
index 7f91186ef1d1..b213870ecb96 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
@@ -1211,12 +1211,7 @@ static int gfx_v8_0_init_microcode(struct amdgpu_device *adev)
 out:
 	if (err) {
 		dev_err(adev->dev, "gfx8: Failed to load firmware %s gfx firmware\n", chip_name);
-		amdgpu_ucode_release(&adev->gfx.pfp_fw);
-		amdgpu_ucode_release(&adev->gfx.me_fw);
-		amdgpu_ucode_release(&adev->gfx.ce_fw);
-		amdgpu_ucode_release(&adev->gfx.rlc_fw);
-		amdgpu_ucode_release(&adev->gfx.mec_fw);
-		amdgpu_ucode_release(&adev->gfx.mec2_fw);
+		gfx_v8_0_free_microcode(adev);
 	}
 	return err;
 }
-- 
2.47.3

Re: [PATCH] drm/amdgpu/gfx8: fix register_list_format leak in gfx_v8_0_init_microcode

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Missing bounds checking and potential integer overflow in firmware header parsing leading to a heap buffer overflow.
--

commit 85ca74ba24e5f9c96f4d94f3bcb428aa05e58c7a
Author: Willian Oliveira <williandossantosdeoliveira287@gmail.com>

drm/amdgpu/gfx8: fix register_list_format leak in gfx_v8_0_init_microcode
    
gfx_v8_0_init_microcode() allocates rlc.register_list_format before
loading the MEC firmware.

If loading one of the subsequent firmware files fails, the function
returns through the error path without freeing register_list_format.

Use gfx_v8_0_free_microcode() in the error path so all previously
allocated firmware resources, including register_list_format, are
released.

> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 6cf4279950787..92c56d811975c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -1211,12 +1211,7 @@ static int gfx_v8_0_init_microcode(struct amdgpu_device *adev)
>  out:
>  	if (err) {
>  		dev_err(adev->dev, "gfx8: Failed to load firmware %s gfx firmware\n", chip_name);
> -		amdgpu_ucode_release(&adev->gfx.pfp_fw);
> -		amdgpu_ucode_release(&adev->gfx.me_fw);
> -		amdgpu_ucode_release(&adev->gfx.ce_fw);
> -		amdgpu_ucode_release(&adev->gfx.rlc_fw);
> -		amdgpu_ucode_release(&adev->gfx.mec_fw);
> -		amdgpu_ucode_release(&adev->gfx.mec2_fw);
> +		gfx_v8_0_free_microcode(adev);

[Severity: Critical]
This is a pre-existing issue, but while reviewing this function's error
handling, I noticed a potential integer overflow in the allocation of
register_list_format higher up in gfx_v8_0_init_microcode():

	adev->gfx.rlc.reg_list_format_size_bytes =
			le32_to_cpu(rlc_hdr->reg_list_format_size_bytes);
	adev->gfx.rlc.reg_list_size_bytes =
			le32_to_cpu(rlc_hdr->reg_list_size_bytes);

	adev->gfx.rlc.register_list_format =
			kmalloc(adev->gfx.rlc.reg_list_format_size_bytes +
					adev->gfx.rlc.reg_list_size_bytes, GFP_KERNEL);

Are we sure reg_list_format_size_bytes and reg_list_size_bytes can't overflow
when added together?

If a maliciously crafted RLC firmware provides large values for these two
variables, the 32-bit addition could wrap around, resulting in a very small
allocation.

Because the subsequent loops iterate over the original un-overflowed values
like this:

	for (i = 0 ; i < (adev->gfx.rlc.reg_list_format_size_bytes >> 2); i++)
		adev->gfx.rlc.register_list_format[i] = le32_to_cpu(tmp[i]);

could this cause out-of-bounds writes into the undersized heap buffer?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260905133405.3733-1-williandossantosdeoliveira287@gmail.com?part=1

Re: [PATCH] drm/amdgpu/gfx8: fix register_list_format leak in gfx_v8_0_init_microcode

[Alex Deucher]

Applied.  Thanks!

On Sat, Sep 5, 2026 at 12:00 PM Willian Oliveira
<williandossantosdeoliveira287@gmail.com> wrote:
>
> gfx_v8_0_init_microcode() allocates rlc.register_list_format before
> loading the MEC firmware.
>
> If loading one of the subsequent firmware files fails, the function
> returns through the error path without freeing register_list_format.
>
> Use gfx_v8_0_free_microcode() in the error path so all previously
> allocated firmware resources, including register_list_format, are
> released.
>
> Signed-off-by: Willian Oliveira <williandossantosdeoliveira287@gmail.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c | 7 +------
>  1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> index 7f91186ef1d1..b213870ecb96 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v8_0.c
> @@ -1211,12 +1211,7 @@ static int gfx_v8_0_init_microcode(struct amdgpu_device *adev)
>  out:
>         if (err) {
>                 dev_err(adev->dev, "gfx8: Failed to load firmware %s gfx firmware\n", chip_name);
> -               amdgpu_ucode_release(&adev->gfx.pfp_fw);
> -               amdgpu_ucode_release(&adev->gfx.me_fw);
> -               amdgpu_ucode_release(&adev->gfx.ce_fw);
> -               amdgpu_ucode_release(&adev->gfx.rlc_fw);
> -               amdgpu_ucode_release(&adev->gfx.mec_fw);
> -               amdgpu_ucode_release(&adev->gfx.mec2_fw);
> +               gfx_v8_0_free_microcode(adev);
>         }
>         return err;
>  }
> --
> 2.47.3
>