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 <[email protected]> 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/[email protected]?part=1
