Re: [PATCH v4 4/5] drm/nouveau/gsp/r570: Start saving comptag backing stores

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] nouveau: integer truncation allows heap overflow via GSP compbit size
--

commit 9c4d2d77f428ab0f70d6ac1a683c604ade884fd2
Author: Lyude Paul <lyude@redhat.com>

drm/nouveau/gsp/r570: Start saving comptag backing stores

This commit implements support for saving and restoring comptag backing
stores during the OpenRM fbsr process. It fetches the required space for the
compbit backing stores from the GSP and adds it to the memory allocated for
fbsr.

> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/rm/r570/fbsr.c
[ ... ]
> @@ -26,6 +26,23 @@ r570_fbsr_suspend_channels(struct nvkm_gsp *gsp, bool suspend)
>  	return nvkm_gsp_rm_ctrl_wr(&gsp->internal.device.subdevice, ctrl);
>  }
>  
> +static int
> +r570_fb_get_compbit_store_size(struct nvkm_gsp *gsp, u64 *size)
> +{
> +	NV0080_CTRL_FB_GET_COMPBIT_STORE_INFO_PARAMS *ctrl;
> +
> +	ctrl = nvkm_gsp_rm_ctrl_rd(&gsp->internal.device.object,
> +				   NV0080_CTRL_CMD_FB_GET_COMPBIT_STORE_INFO,
> +				   sizeof(*ctrl));
> +	if (IS_ERR(ctrl))
> +		return PTR_ERR(ctrl);
> +
> +	*size = ctrl->Size;

[Severity: High]
Does this code need to validate the untrusted 64-bit value returned from the
GSP firmware?

If a compromised or malicious GSP firmware provides a crafted value, it can be
used to exploit integer truncations during allocation.

> +
> +	nvkm_gsp_rm_ctrl_done(&gsp->internal.device.object, ctrl);
> +	return 0;
> +}
[ ... ]
> @@ -174,6 +197,8 @@ r570_fbsr_suspend(struct nvkm_gsp *gsp)
>  	size  = gsp->fb.heap.size;
>  	size += gsp->fb.rsvd_size;
>  	size += gsp->fb.bios.vga_workspace.size;
> +	size += compbit_store_size;

[Severity: High]
Can this unvalidated addition allow an attacker to craft a specific size
that perfectly aligns for integer truncation in the allocator?

When nvkm_gsp_sg() receives this size, it computes pages and passes it to
sg_alloc_table():

nvkm_gsp_sg(device, size, &gsp->sr.fbsr)
   const u64 pages = DIV_ROUND_UP(size, PAGE_SIZE);
   ...
   ret = sg_alloc_table(sgt, pages, GFP_KERNEL);

Since sg_alloc_table() takes a 32-bit unsigned int nents, a crafted size
(like 4TB + 1 bytes) would truncate to a tiny non-zero value, allowing a
massive allocation request to succeed by allocating only a few pages.

Additionally, when this size reaches r535_fbsr_memlist():

r535_fbsr_memlist()
   const u32 pages = size / GSP_PAGE_SIZE;
   ...
   rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_ALLOC_MEMORY,
                          sizeof(*rpc) + pages * sizeof(rpc->pteDesc.pte_pde[0]));

The 32-bit truncation results in a 0-PTE buffer allocation. The subsequent loop
then writes PTEs based on the actual SG table length, causing an out-of-bounds
heap write.

> +
>  	nvkm_debug(subdev, "fbsr: size: 0x%llx bytes\n", size);
>  
>  	ret = nvkm_gsp_sg(device, size, &gsp->sr.fbsr);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260916223358.507351-1-lyude@redhat.com?part=4