Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Out-of-bounds reads in `check_atom_bios` when validating VBIOS headers, made directly reachable via the new user-supplied firmware file fallback. -- commit a2fe12b488b874804f022bf152336e0c586847dc Author: Oz Tiram <[email protected]> drm/amd/amdgpu: add firmware file fallback for APU VBIOS discovery This commit adds a fifth fallback method to discover an APU VBIOS by requesting a firmware file when all other methods fail. The fallback uses request_firmware() and allocates the VBIOS buffer using kmemdup() with the exact firmware size. > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c > index aa039e148a5ee..86064c753b099 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_bios.c [ ... ] > @@ -457,6 +458,28 @@ static bool amdgpu_get_bios_apu(struct amdgpu_device > *adev) > goto success; > } > > + { > + const struct firmware *fw; > + char fw_name[32]; > + size_t fw_size; > + > + snprintf(fw_name, sizeof(fw_name), "amdgpu/%04x_%04x.bin", > + adev->pdev->vendor, adev->pdev->device); > + if (request_firmware(&fw, fw_name, adev->dev) == 0) { > + adev->bios = kmemdup(fw->data, fw->size, GFP_KERNEL); > + fw_size = fw->size; > + release_firmware(fw); > + if (!adev->bios || !check_atom_bios(adev, fw_size)) { [Severity: High] This isn't a bug introduced by this patch, but allocating the buffer to the exact firmware size via kmemdup() exposes existing out-of-bounds reads in check_atom_bios(). Looking at check_atom_bios(), the initial size check is: if (!bios || size < 0x49) { ... return false; } But then it reads bios[0x49]: bios_header_start = bios[0x48] | (bios[0x49] << 8); If fw_size is exactly 0x49, bios[0x49] reads 1 byte past the end of the kmemdup() buffer. Similarly, check_atom_bios() later checks size against tmp before reading 4 bytes for the ATOM signature: tmp = bios_header_start + 4; if (size < tmp) { ... return false; } if (!memcmp(bios + tmp, "ATOM", 4) || !memcmp(bios + tmp, "MOTA", 4)) { If size is exactly tmp (or up to tmp + 3), memcmp() will read up to 4 bytes past the end of the allocation. Could a malformed firmware file loaded through this new path trigger KASAN slab-out-of-bounds warnings or page faults due to these bounds checks? > + amdgpu_bios_release(adev); > + } else { > + adev->bios_size = fw_size; > + dev_info(adev->dev, "Fetched VBIOS from > firmware file %s\n", > + fw_name); > + goto success; > + } > + } > + } > + > dev_err(adev->dev, "Unable to locate a BIOS ROM\n"); > return false; > -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
