[PATCH] drm/radeon: check bo_va->bo is non-NULL before using it

Christian König christian.koenig at amd.com
Tue Jun 25 12:45:28 UTC 2024


Am 25.06.24 um 14:41 schrieb Pierre-Eric Pelloux-Prayer:
> The call to radeon_vm_clear_freed might clear bo_va->bo, so
> we have to check it before dereferencing it.
>
> Signed-off-by: Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer at amd.com>
> ---
>   drivers/gpu/drm/radeon/radeon_gem.c | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
> index 3fec3acdaf28..0cf1a72091b7 100644
> --- a/drivers/gpu/drm/radeon/radeon_gem.c
> +++ b/drivers/gpu/drm/radeon/radeon_gem.c
> @@ -641,8 +641,17 @@ static void radeon_gem_va_update_vm(struct radeon_device *rdev,
>   	if (r)
>   		goto error_unlock;
>   
> -	if (bo_va->it.start)
> +	if (bo_va->it.start) {
> +		if (bo_va->bo == NULL) {

In the kernel you should probably use !bo_va->bo instead.

> +			/* Buggy userspace can try to use RADEON_VA_UNMAP after
> +			 * closing the BO. In this case, radeon_vm_clear_freed
> +			 * will unset bo_va->bo.
> +			 */

I think you can drop that comment, it's perfectly valid for userspace to 
unmap a region after closing the BO.

It's just that the radeon VM support was never designed very well.

Regards,
Christian.

> +			r = -ENOENT;
> +			goto error_unlock;
> +		}
>   		r = radeon_vm_bo_update(rdev, bo_va, bo_va->bo->tbo.resource);
> +	}
>   
>   error_unlock:
>   	mutex_unlock(&bo_va->vm->mutex);



More information about the amd-gfx mailing list