[PATCH] drm/nouveau: validate required NET_img regions

[PATCH] drm/nouveau: validate required NET_img regions

[Slavin Liu]

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.

Detected by static analysis and reviewed with AI-assisted source auditing.

Fixes: c4bdac754ca0 ("drm/nouveau/gr/ga102: initial support")
Assisted-by: LLM
Signed-off-by: Slavin Liu <bolin.liu@seu.edu.cn>
---
 drivers/gpu/drm/nouveau/nvkm/engine/gr/ga102.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/gr/ga102.c b/drivers/gpu/drm/nouveau/nvkm/engine/gr/ga102.c
index 2b51f1d0c281..bfd1e00537b8 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,
@@ -324,7 +329,7 @@ ga102_gr_load(struct gf100_gr *gr, int ver, const struct gf100_gr_fwif *fwif)
 					    fw->data + fecs_data->data_offset,
 						       fecs_data->data_size);
 	if (ret)
-		return ret;
+		goto out_firmware;
 
 	ret = nvkm_acr_lsfw_load_bl_sig_net(subdev, &gr->gpccs.falcon, NVKM_ACR_LSF_GPCCS,
 					    "gr/gpccs_", ver, fwif->gpccs,
@@ -333,12 +338,13 @@ ga102_gr_load(struct gf100_gr *gr, int ver, const struct gf100_gr_fwif *fwif)
 					    fw->data + gpccs_data->data_offset,
 						       gpccs_data->data_size);
 	if (ret)
-		return ret;
+		goto out_firmware;
 
 	gr->firmware = true;
 
+ out_firmware:
 	nvkm_firmware_put(fw);
-	return 0;
+	return ret;
 }
 
 static const struct gf100_gr_fwif

Re: [PATCH] drm/nouveau: validate required NET_img regions

[sashiko-bot]

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 <bolin.liu@seu.edu.cn>

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/20260913125228.110071-1-bolin.liu@seu.edu.cn?part=1