Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Missing bounds validation on firmware region counts, offsets, and dimensions allows out-of-bounds memory accesses and integer overflows if the firmware file is malformed or truncated. -- commit 8eea807eacef8758484282158b451c9cf174e015 Author: Slavin Liu <[email protected]> drm/nouveau: validate required NET_img regions The NET_img parser can finish without all FECS and GPCCS regions. Reject missing required regions before computing their data addresses, and release the firmware on both validation and ACR loading failures. > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/gr/ga102.c > b/drivers/gpu/drm/nouveau/nvkm/engine/gr/ga102.c > index 2b51f1d0c281f..bfd1e00537b89 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/engine/gr/ga102.c > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/gr/ga102.c > @@ -317,6 +317,11 @@ ga102_gr_load(struct gf100_gr *gr, int ver, const struct > gf100_gr_fwif *fwif) > } > } > > + if (!fecs_inst || !fecs_data || !gpccs_inst || !gpccs_data) { > + ret = -EINVAL; > + goto out_firmware; > + } > + > ret = nvkm_acr_lsfw_load_bl_sig_net(subdev, &gr->fecs.falcon, > NVKM_ACR_LSF_FECS, > "gr/fecs_", ver, fwif->fecs, > fw->data + fecs_inst->data_offset, > fecs_inst->data_size, > fw->data + fecs_data->data_offset, > fecs_data->data_size); [Severity: High] This is a pre-existing issue, but while validating that the required regions are present, does the code lack bounds checking for the firmware file itself? If the user-provided firmware is malformed or truncated, the earlier loop in ga102_gr_load() could read out of bounds because net->header.regions and fw->size are never validated: for (i = 0; i < net->header.regions; i++) { const struct netlist_region *reg = &net->regions[i]; struct nvkm_blob blob = { .data = (void *)fw->data + reg->data_offset, .size = reg->data_size, }; Additionally, can this lead to integer truncation when the unvalidated fecs_inst->data_size is passed to nvkm_acr_lsfw_load_bl_sig_net() here? drivers/gpu/drm/nouveau/nvkm/subdev/acr/lsfw.c:nvkm_acr_lsfw_load_bl_sig_net() { ... lsfw->app_resident_code_size = ALIGN(inst->size, 256); ... if (!(lsfw->img.data = kzalloc(lsfw->img.size, GFP_KERNEL))) { ... memcpy(lsfw->img.data + ..., inst->data, inst->size); ... } Could an overflow during the ALIGN() or size calculation cause an undersized allocation followed by a heap buffer overflow during the memcpy? Similarly, would this unvalidated blob->size cause an integer overflow in the allocation size and a subsequent out-of-bounds write loop in gk20a_gr_av_to_init_()? drivers/gpu/drm/nouveau/nvkm/engine/gr/gk20a.c:gk20a_gr_av_to_init_() { ... nent = (blob->size / sizeof(struct gk20a_fw_av)); pack = vzalloc((sizeof(*pack) * 2) + (sizeof(*init) * (nent + 1))); ... for (i = 0; i < nent; i++) { struct gf100_gr_init *ent = &init[i]; ... } [ ... ] -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
