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);
> }
>
