[PATCH 1/2] [v3] drm/nouveau: fix several DMA buffer leaks

[Danilo Krummrich]

On 2/3/24 00:06, Timur Tabi wrote:
> Nouveau manages GSP-RM DMA buffers with nvkm_gsp_mem objects.  Several of
> these buffers are never dealloced.  Some of them can be deallocated
> right after GSP-RM is initialized, but the rest need to stay until the
> driver unloads.
> 
> Also futher bullet-proof these objects by poisoning the buffer and
> clearing the nvkm_gsp_mem object when it is deallocated.  Poisoning
> the buffer should trigger an error (or crash) from GSP-RM if it tries
> to access the buffer after we've deallocated it, because we were wrong
> about when it is safe to deallocate.
> 
> Finally, change the mem->size field to a size_t because that's the same
> type that dma_alloc_coherent expects.
> 
> Fixes: 176fdcbddfd2 ("drm/nouveau/gsp/r535: add support for booting GSP-RM")
> Signed-off-by: Timur Tabi <ttabi at nvidia.com>

Series applied to drm-misc-fixes, thanks!

> ---
> v3: use size_t
> v2: add buffer poisoning
> 
>   .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h |  2 +-
>   .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 59 ++++++++++++-------
>   2 files changed, 39 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
> index 5c6b8536e31c..3fbc57b16a05 100644
> --- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
> +++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
> @@ -9,7 +9,7 @@
>   #define GSP_PAGE_SIZE  BIT(GSP_PAGE_SHIFT)
>   
>   struct nvkm_gsp_mem {
> -	u32 size;
> +	size_t size;
>   	void *data;
>   	dma_addr_t addr;
>   };
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
> index 17fc429ee50b..a9030eb83b4d 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
> @@ -999,6 +999,32 @@ r535_gsp_rpc_get_gsp_static_info(struct nvkm_gsp *gsp)
>   	return 0;
>   }
>   
> +static void
> +nvkm_gsp_mem_dtor(struct nvkm_gsp *gsp, struct nvkm_gsp_mem *mem)
> +{
> +	if (mem->data) {
> +		/*
> +		 * Poison the buffer to catch any unexpected access from
> +		 * GSP-RM if the buffer was prematurely freed.
> +		 */
> +		memset(mem->data, 0xFF, mem->size);
> +
> +		dma_free_coherent(gsp->subdev.device->dev, mem->size, mem->data, mem->addr);
> +		memset(mem, 0, sizeof(*mem));
> +	}
> +}
> +
> +static int
> +nvkm_gsp_mem_ctor(struct nvkm_gsp *gsp, size_t size, struct nvkm_gsp_mem *mem)
> +{
> +	mem->size = size;
> +	mem->data = dma_alloc_coherent(gsp->subdev.device->dev, size, &mem->addr, GFP_KERNEL);
> +	if (WARN_ON(!mem->data))
> +		return -ENOMEM;
> +
> +	return 0;
> +}
> +
>   static int
>   r535_gsp_postinit(struct nvkm_gsp *gsp)
>   {
> @@ -1026,6 +1052,13 @@ r535_gsp_postinit(struct nvkm_gsp *gsp)
>   
>   	nvkm_inth_allow(&gsp->subdev.inth);
>   	nvkm_wr32(device, 0x110004, 0x00000040);
> +
> +	/* Release the DMA buffers that were needed only for boot and init */
> +	nvkm_gsp_mem_dtor(gsp, &gsp->boot.fw);
> +	nvkm_gsp_mem_dtor(gsp, &gsp->libos);
> +	nvkm_gsp_mem_dtor(gsp, &gsp->rmargs);
> +	nvkm_gsp_mem_dtor(gsp, &gsp->wpr_meta);
> +
>   	return ret;
>   }
>   
> @@ -1760,27 +1793,6 @@ r535_gsp_msg_run_cpu_sequencer(void *priv, u32 fn, void *repv, u32 repc)
>   	return 0;
>   }
>   
> -static void
> -nvkm_gsp_mem_dtor(struct nvkm_gsp *gsp, struct nvkm_gsp_mem *mem)
> -{
> -	if (mem->data) {
> -		dma_free_coherent(gsp->subdev.device->dev, mem->size, mem->data, mem->addr);
> -		mem->data = NULL;
> -	}
> -}
> -
> -static int
> -nvkm_gsp_mem_ctor(struct nvkm_gsp *gsp, u32 size, struct nvkm_gsp_mem *mem)
> -{
> -	mem->size = size;
> -	mem->data = dma_alloc_coherent(gsp->subdev.device->dev, size, &mem->addr, GFP_KERNEL);
> -	if (WARN_ON(!mem->data))
> -		return -ENOMEM;
> -
> -	return 0;
> -}
> -
> -
>   static int
>   r535_gsp_booter_unload(struct nvkm_gsp *gsp, u32 mbox0, u32 mbox1)
>   {
> @@ -2378,6 +2390,11 @@ r535_gsp_dtor(struct nvkm_gsp *gsp)
>   	mutex_destroy(&gsp->cmdq.mutex);
>   
>   	r535_gsp_dtor_fws(gsp);
> +
> +	nvkm_gsp_mem_dtor(gsp, &gsp->shm.mem);
> +	nvkm_gsp_mem_dtor(gsp, &gsp->loginit);
> +	nvkm_gsp_mem_dtor(gsp, &gsp->logintr);
> +	nvkm_gsp_mem_dtor(gsp, &gsp->logrm);
>   }
>   
>   int