Re: [PATCH] drm/amdgpu: unmap GART dma pages before free

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/amdgpu: NULL pointer dereference in amdgpu_gart_table_ram_alloc error path
--

commit cffcb1919743d55f9dc644f758abf99d4187be7c
Author: Michail Tatas <michail.tatas@gmail.com>

drm/amdgpu: unmap GART dma pages before free

This commit unmaps the GART dma pages before freeing them to avoid leaving
the device writing to freed memory.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> index c4c21dbbbdbf8..780fb01530e69 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gart.c
[ ... ]
> @@ -209,6 +209,8 @@ int amdgpu_gart_table_ram_alloc(struct amdgpu_device *adev)
>  error_resv:
>  	amdgpu_bo_unreserve(bo);
>  error:
> +	dma_unmap_page(&adev->pdev->dev, sg_dma_address(sg->sgl), adev->gart.table_size,
> +		       DMA_BIDIRECTIONAL);

[Severity: High]
Can this cause a NULL pointer dereference during memory pressure?

If memory allocation fails earlier in amdgpu_gart_table_ram_alloc(),
sg is set to NULL and execution jumps to the error label:

	sg = kmalloc_obj(*sg);
	if (!sg) {
		ret = -ENOMEM;
		goto error;
	}

The new dma_unmap_page() call unconditionally accesses sg->sgl at the error
label. If the execution arrived here due to the allocation failure above,
dereferencing sg->sgl will crash.

Additionally, if sg_alloc_table() fails, sg is allocated but sg->sgl may be
uninitialized when accessed here.

>  	amdgpu_bo_unref(&bo);
>  	if (sg) {
>  		sg_free_table(sg);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/arKhDFCSoPDNYaxX@michalis-linux?part=1