[Mesa-dev] [PATCH 7/8] radv/amdgpu: Use reference counting for bos.

[Nicolai Hähnle]

On 05.02.2017 12:43, Bas Nieuwenhuizen wrote:
> Per the Vulkan spec, memory objects may be deleted before the buffers
> and images using them are deleted, although those resources then
> cannot be used except for deletion themselves.
>
> For the virtual buffers, we need to access them on resource destruction
> to unmap the regions, so this results in a use-after-free. Implement
> reference counting to avoid this.

So there's really no requirement to remove the bindings before deleting 
the memory objects? Sucks :/

Cheers,
Nicolai

>
> Signed-off-by: Bas Nieuwenhuizen <basni at google.com>
> ---
>  src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c | 10 ++++++++++
>  src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.h |  1 +
>  2 files changed, 11 insertions(+)
>
> diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c
> index d5bce304510..6519b096fcc 100644
> --- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c
> +++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.c
> @@ -34,6 +34,11 @@
>  #include <amdgpu_drm.h>
>  #include <inttypes.h>
>
> +#include "util/u_atomic.h"
> +
> +
> +static void radv_amdgpu_winsys_bo_destroy(struct radeon_winsys_bo *_bo);
> +
>  static void
>  radv_amdgpu_winsys_virtual_map(struct radv_amdgpu_winsys_bo *bo,
>                                 const struct radv_amdgpu_map_range *range)
> @@ -43,6 +48,7 @@ radv_amdgpu_winsys_virtual_map(struct radv_amdgpu_winsys_bo *bo,
>  	if (!range->bo)
>  		return; /* TODO: PRT mapping */
>
> +	p_atomic_inc(&range->bo->ref_count);
>  	int r = amdgpu_bo_va_op(range->bo->bo, range->bo_offset, range->size,
>  	                        range->offset + bo->va, 0, AMDGPU_VA_OP_MAP);
>  	if (r)
> @@ -62,6 +68,7 @@ radv_amdgpu_winsys_virtual_unmap(struct radv_amdgpu_winsys_bo *bo,
>  	                        range->offset + bo->va, 0, AMDGPU_VA_OP_UNMAP);
>  	if (r)
>  		abort();
> +	radv_amdgpu_winsys_bo_destroy((struct radeon_winsys_bo *)range->bo);
>  }
>
>  static void
> @@ -193,6 +200,8 @@ static void radv_amdgpu_winsys_bo_destroy(struct radeon_winsys_bo *_bo)
>  {
>  	struct radv_amdgpu_winsys_bo *bo = radv_amdgpu_winsys_bo(_bo);
>
> +	if (p_atomic_dec_return(&bo->ref_count))
> +		return;
>  	if (bo->is_virtual) {
>  		for (uint32_t i = 0; i < bo->range_count; ++i) {
>  			radv_amdgpu_winsys_virtual_unmap(bo, bo->ranges + i);
> @@ -254,6 +263,7 @@ radv_amdgpu_winsys_bo_create(struct radeon_winsys *_ws,
>  	bo->size = size;
>  	bo->ws = ws;
>  	bo->is_virtual = !!(flags & RADEON_FLAG_VIRTUAL);
> +	bo->ref_count = 1;
>
>  	if (flags & RADEON_FLAG_VIRTUAL) {
>  		bo->ranges = realloc(NULL, sizeof(struct radv_amdgpu_map_range));
> diff --git a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.h b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.h
> index c7d484bc8dd..4512e76b333 100644
> --- a/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.h
> +++ b/src/amd/vulkan/winsys/amdgpu/radv_amdgpu_bo.h
> @@ -45,6 +45,7 @@ struct radv_amdgpu_winsys_bo {
>  	uint64_t size;
>  	struct radv_amdgpu_winsys *ws;
>  	bool is_virtual;
> +	int ref_count;
>
>  	union {
>  		/* physical bo */
>