[PATCH v1] drm/amdgpu: fix framebuffer memory use after free

[Michel Dänzer]

On 2021-06-17 10:18 a.m., Lukasz Bartosik wrote:
> With option CONFIG_DEBUG_LIST enabled the kernel logs show list_add
> corruption warning. The warning originates from drm_framebuffer_init()
> function which adds framebuffer to a framebuffers list and is called by
> amdgpu_display_gem_fb_verify_and_init().
> If amdgpu_display_gem_fb_verify_and_init() encounters an error after
> calling drm_framebuffer_init() then framebuffer memory is released
> in amdgpu_display_user_framebuffer_create() without removing framebuffer
> from the list where it was added. Reuse of that memory by any other
> party cause corruption of the framebuffers linked list. This fix removes
> framebuffer from the linked list and unregisters it in case of failure.
> 
> [...]
> 
> Fixes: 6eed95b00b45 ("drm/amd/display: Store tiling_flags in the framebuffer.")

I didn't realize there was already an issue before f258907fdd835e "drm/amdgpu: Verify bo size can fit framebuffer size on init.". Looking at 
the Git history again, I agree there's already at least a theoretical issue in 5.11, though I suspect it's harder to hit in practice.


> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> index c13985fb35be..933190281b91 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c
> @@ -1085,14 +1085,17 @@ int amdgpu_display_gem_fb_verify_and_init(
>  			    mode_cmd->modifier[0]);
>  
>  		ret = -EINVAL;
> -		goto err;
> +		goto err_fb_cleanup;
>  	}
>  
>  	ret = amdgpu_display_framebuffer_init(dev, rfb, mode_cmd, obj);
>  	if (ret)
> -		goto err;
> +		goto err_fb_cleanup;
>  
>  	return 0;
> +err_fb_cleanup:
> +	drm_framebuffer_unregister_private(&rfb->base);
> +	drm_framebuffer_cleanup(&rfb->base);
>  err:
>  	drm_dbg_kms(dev, "Failed to verify and init gem fb: %d\n", ret);
>  	rfb->base.obj[0] = NULL;
> 

There's a similar issue in amdgpu_display_gem_fb_init. https://patchwork.freedesktop.org/patch/439542/ fixes that as well, and seems simpler (though I'm biased obviously :).


Neither patch can be trivially cherry picked for fixing the issue in 5.11/5.12 due to f258907fdd835e.


-- 
Earthling Michel Dänzer               |               https://redhat.com
Libre software enthusiast             |             Mesa and X developer