[PATCH 1/4] drm/amdgpu: fix user fence handling once more

[zhoucm1]

On 2016年06月30日 16:53, Christian König wrote:
> From: Christian König <christian.koenig at amd.com>
>
> Same problem as with the VM page tables. The user fence address must be
> determined before the job is scheduled, not when the IB is executed.
>
> This fixes a security problem where user fences could be used to overwrite
> any part of VRAM.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>

Reviewed-by: Chunming Zhou <david1.zhou at amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h     |  3 +--
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 12 ++++++------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c  |  7 ++-----
>   drivers/gpu/drm/amd/amdgpu/amdgpu_job.c |  1 -
>   4 files changed, 9 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index bf714a5..0f18c67 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1277,8 +1277,7 @@ struct amdgpu_job {
>   	uint32_t		oa_base, oa_size;
>   
>   	/* user fence handling */
> -	struct amdgpu_bo	*uf_bo;
> -	uint32_t		uf_offset;
> +	uint64_t		uf_addr;
>   	uint64_t		uf_sequence;
>   
>   };
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index a3d7d13..9f148ba 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -217,11 +217,8 @@ int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, void *data)
>   	if (ret)
>   		goto free_all_kdata;
>   
> -	if (p->uf_entry.robj) {
> -		p->job->uf_bo = amdgpu_bo_ref(p->uf_entry.robj);
> -		p->job->uf_offset = uf_offset;
> -	}
> -
> +	if (p->uf_entry.robj)
> +		p->job->uf_addr = uf_offset;
>   	kfree(chunk_array);
>   	return 0;
>   
> @@ -500,6 +497,9 @@ static int amdgpu_cs_parser_bos(struct amdgpu_cs_parser *p,
>   		}
>   	}
>   
> +	if (p->uf_entry.robj)
> +		p->job->uf_addr += amdgpu_bo_gpu_offset(p->uf_entry.robj);
> +
>   error_validate:
>   	if (r) {
>   		amdgpu_vm_move_pt_bos_in_lru(p->adev, &fpriv->vm);
> @@ -762,7 +762,7 @@ static int amdgpu_cs_ib_fill(struct amdgpu_device *adev,
>   	}
>   
>   	/* UVD & VCE fw doesn't support user fences */
> -	if (parser->job->uf_bo && (
> +	if (parser->job->uf_addr && (
>   	    parser->job->ring->type == AMDGPU_RING_TYPE_UVD ||
>   	    parser->job->ring->type == AMDGPU_RING_TYPE_VCE))
>   		return -EINVAL;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index 34e3542..0bf6c1b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -203,11 +203,8 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned num_ibs,
>   	}
>   
>   	/* wrap the last IB with fence */
> -	if (job && job->uf_bo) {
> -		uint64_t addr = amdgpu_bo_gpu_offset(job->uf_bo);
> -
> -		addr += job->uf_offset;
> -		amdgpu_ring_emit_fence(ring, addr, job->uf_sequence,
> +	if (job && job->uf_addr) {
> +		amdgpu_ring_emit_fence(ring, job->uf_addr, job->uf_sequence,
>   				       AMDGPU_FENCE_FLAG_64BIT);
>   	}
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> index b50a845..87b75d7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
> @@ -91,7 +91,6 @@ static void amdgpu_job_free_resources(struct amdgpu_job *job)
>   		amdgpu_ib_free(job->adev, &job->ibs[i], f);
>   	fence_put(job->fence);
>   
> -	amdgpu_bo_unref(&job->uf_bo);
>   	amdgpu_sync_free(&job->sync);
>   }
>