Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing bounds checking on firmware blob offsets allows out-of-bounds kernel memory reads. -- commit 69b65113e7205835c286bfc4a8d86163010bab77 Author: Dawei Feng <[email protected]> nouveau/firmware: fix memory leak on BL load failure If loading the HS bootloader blob fails, nvkm_falcon_fw_ctor_hs() returns immediately, skipping the common cleanup path and leaking firmware state. This patch routes the load failure to the 'done' label to properly clean up the partially initialized state and clears the original blob pointer. > diff --git a/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c > b/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c > index 4e8b3f1c7e25d..71f55c5b08374 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c > +++ b/drivers/gpu/drm/nouveau/nvkm/falcon/fw.c > @@ -278,10 +278,11 @@ nvkm_falcon_fw_ctor_hs(const struct nvkm_falcon_fw_func > *func, const char *name, > > if (bl) { > nvkm_firmware_put(blob); > + blob = NULL; > > ret = nvkm_firmware_load_name(subdev, bl, "", ver, &blob); > if (ret) > - return ret; > + goto done; > > hdr = nvfw_bin_hdr(subdev, blob->data); > desc = nvfw_bl_desc(subdev, blob->data + hdr->header_offset); [Severity: High] This isn't a bug introduced by this patch, but I noticed a pre-existing issue with bounds checking when parsing the firmware blobs. Does this code safely handle potentially malformed firmware files? When the firmware headers are parsed, offsets and sizes like hdr->data_offset, desc->code_off, and desc->code_size are extracted without being validated against the actual blob->size. Just after this hunk in nvkm_falcon_fw_ctor_hs(), the unvalidated values are used to calculate the source pointer for kmemdup(): fw->boot = kmemdup(blob->data + hdr->data_offset + desc->code_off, fw->boot_size, GFP_KERNEL); Could this lead to an out-of-bounds read on the kernel heap if a corrupted firmware blob is provided? If the read crosses into unmapped memory, it might trigger a kernel oops. Similarly, earlier in the same function, nvkm_falcon_fw_ctor() is called with unchecked offsets: ret = nvkm_falcon_fw_ctor(func, name, subdev->device, bl != NULL, blob->data + hdr->data_offset, hdr->data_size, falcon, fw); Would it be helpful to add validation checks against blob->size before these header values are consumed? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
