Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing bounds checking when parsing firmware headers allows out-of-bounds memory accesses. -- commit 72846afc6e077321cb6e56447e8430295e342e68 Author: Dawei Feng <[email protected]> nouveau/firmware: fix memory leak on BL load failure This patch addresses a memory leak when loading the HS bootloader blob fails. It routes the failure to the done label so that the partially initialized firmware state can be properly cleaned up rather than returning immediately. > diff --git a/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c > b/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c > --- a/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c > +++ b/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c [ ... ] > @@ -277,19 +277,20 @@ nvkm_falcon_fw_ctor_hs(const struct nvkm_falcon_fw_func > *func, const char *name, > fw->dmem_sign = loc - lhdr->data_dma_base; > > if (bl) { > - nvkm_firmware_put(blob); > + const struct firmware *blob_bl; > > - ret = nvkm_firmware_load_name(subdev, bl, "", ver, &blob); > + ret = nvkm_firmware_load_name(subdev, bl, "", ver, &blob_bl); > if (ret) > - return ret; > + goto done; > > - hdr = nvfw_bin_hdr(subdev, blob->data); > - desc = nvfw_bl_desc(subdev, blob->data + hdr->header_offset); > + hdr = nvfw_bin_hdr(subdev, blob_bl->data); > + desc = nvfw_bl_desc(subdev, blob_bl->data + hdr->header_offset); > > fw->boot_addr = desc->start_tag << 8; > fw->boot_size = desc->code_size; > - fw->boot = kmemdup(blob->data + hdr->data_offset + > desc->code_off, > + fw->boot = kmemdup(blob_bl->data + hdr->data_offset + > desc->code_off, > fw->boot_size, GFP_KERNEL); [Severity: High] This isn't a bug introduced by this patch, but does the firmware parsing code in nvkm_falcon_fw_ctor_hs() lack bounds checking? The offsets (such as hdr->header_offset, hdr->data_offset, and desc->code_off) and sizes (like desc->code_size) are read directly from the firmware blob. Because functions like nvfw_bin_hdr() and nvfw_bl_desc() do not take the buffer size as an argument, they cannot perform structural validation. Could a malformed or corrupted firmware file cause an out-of-bounds memory access when kmemdup() tries to copy data, or trigger an unusually large allocation if desc->code_size is excessively large without being validated against the total firmware size (blob_bl->size)? > + nvkm_firmware_put(blob_bl); > if (!fw->boot) > ret = -ENOMEM; > } else { -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
