[PATCH 1/4] drm/radeon: make VCE handle check more strict
Alex Deucher
alexdeucher at gmail.com
Thu May 7 09:29:58 PDT 2015
On Thu, May 7, 2015 at 9:19 AM, Christian König <deathsimple at vodafone.de> wrote:
> From: Christian König <christian.koenig at amd.com>
>
> Invalid handles can crash the hw.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> CC: stable at vger.kernel.org
Applied the series to my -fixes tree.
Alex
> ---
> drivers/gpu/drm/radeon/radeon_vce.c | 65 +++++++++++++++++++++++++++----------
> 1 file changed, 48 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_vce.c b/drivers/gpu/drm/radeon/radeon_vce.c
> index 24f849f..0de5711 100644
> --- a/drivers/gpu/drm/radeon/radeon_vce.c
> +++ b/drivers/gpu/drm/radeon/radeon_vce.c
> @@ -493,18 +493,27 @@ int radeon_vce_cs_reloc(struct radeon_cs_parser *p, int lo, int hi,
> *
> * @p: parser context
> * @handle: handle to validate
> + * @allocated: allocated a new handle?
> *
> * Validates the handle and return the found session index or -EINVAL
> * we we don't have another free session index.
> */
> -int radeon_vce_validate_handle(struct radeon_cs_parser *p, uint32_t handle)
> +static int radeon_vce_validate_handle(struct radeon_cs_parser *p,
> + uint32_t handle, bool *allocated)
> {
> unsigned i;
>
> + *allocated = false;
> +
> /* validate the handle */
> for (i = 0; i < RADEON_MAX_VCE_HANDLES; ++i) {
> - if (atomic_read(&p->rdev->vce.handles[i]) == handle)
> + if (atomic_read(&p->rdev->vce.handles[i]) == handle) {
> + if (p->rdev->vce.filp[i] != p->filp) {
> + DRM_ERROR("VCE handle collision detected!\n");
> + return -EINVAL;
> + }
> return i;
> + }
> }
>
> /* handle not found try to alloc a new one */
> @@ -512,6 +521,7 @@ int radeon_vce_validate_handle(struct radeon_cs_parser *p, uint32_t handle)
> if (!atomic_cmpxchg(&p->rdev->vce.handles[i], 0, handle)) {
> p->rdev->vce.filp[i] = p->filp;
> p->rdev->vce.img_size[i] = 0;
> + *allocated = true;
> return i;
> }
> }
> @@ -529,10 +539,10 @@ int radeon_vce_validate_handle(struct radeon_cs_parser *p, uint32_t handle)
> int radeon_vce_cs_parse(struct radeon_cs_parser *p)
> {
> int session_idx = -1;
> - bool destroyed = false;
> + bool destroyed = false, created = false, allocated = false;
> uint32_t tmp, handle = 0;
> uint32_t *size = &tmp;
> - int i, r;
> + int i, r = 0;
>
> while (p->idx < p->chunk_ib->length_dw) {
> uint32_t len = radeon_get_ib_value(p, p->idx);
> @@ -540,18 +550,21 @@ int radeon_vce_cs_parse(struct radeon_cs_parser *p)
>
> if ((len < 8) || (len & 3)) {
> DRM_ERROR("invalid VCE command length (%d)!\n", len);
> - return -EINVAL;
> + r = -EINVAL;
> + goto out;
> }
>
> if (destroyed) {
> DRM_ERROR("No other command allowed after destroy!\n");
> - return -EINVAL;
> + r = -EINVAL;
> + goto out;
> }
>
> switch (cmd) {
> case 0x00000001: // session
> handle = radeon_get_ib_value(p, p->idx + 2);
> - session_idx = radeon_vce_validate_handle(p, handle);
> + session_idx = radeon_vce_validate_handle(p, handle,
> + &allocated);
> if (session_idx < 0)
> return session_idx;
> size = &p->rdev->vce.img_size[session_idx];
> @@ -561,6 +574,13 @@ int radeon_vce_cs_parse(struct radeon_cs_parser *p)
> break;
>
> case 0x01000001: // create
> + created = true;
> + if (!allocated) {
> + DRM_ERROR("Handle already in use!\n");
> + r = -EINVAL;
> + goto out;
> + }
> +
> *size = radeon_get_ib_value(p, p->idx + 8) *
> radeon_get_ib_value(p, p->idx + 10) *
> 8 * 3 / 2;
> @@ -578,12 +598,12 @@ int radeon_vce_cs_parse(struct radeon_cs_parser *p)
> r = radeon_vce_cs_reloc(p, p->idx + 10, p->idx + 9,
> *size);
> if (r)
> - return r;
> + goto out;
>
> r = radeon_vce_cs_reloc(p, p->idx + 12, p->idx + 11,
> *size / 3);
> if (r)
> - return r;
> + goto out;
> break;
>
> case 0x02000001: // destroy
> @@ -594,7 +614,7 @@ int radeon_vce_cs_parse(struct radeon_cs_parser *p)
> r = radeon_vce_cs_reloc(p, p->idx + 3, p->idx + 2,
> *size * 2);
> if (r)
> - return r;
> + goto out;
> break;
>
> case 0x05000004: // video bitstream buffer
> @@ -602,36 +622,47 @@ int radeon_vce_cs_parse(struct radeon_cs_parser *p)
> r = radeon_vce_cs_reloc(p, p->idx + 3, p->idx + 2,
> tmp);
> if (r)
> - return r;
> + goto out;
> break;
>
> case 0x05000005: // feedback buffer
> r = radeon_vce_cs_reloc(p, p->idx + 3, p->idx + 2,
> 4096);
> if (r)
> - return r;
> + goto out;
> break;
>
> default:
> DRM_ERROR("invalid VCE command (0x%x)!\n", cmd);
> - return -EINVAL;
> + r = -EINVAL;
> + goto out;
> }
>
> if (session_idx == -1) {
> DRM_ERROR("no session command at start of IB\n");
> - return -EINVAL;
> + r = -EINVAL;
> + goto out;
> }
>
> p->idx += len / 4;
> }
>
> - if (destroyed) {
> - /* IB contains a destroy msg, free the handle */
> + if (allocated && !created) {
> + DRM_ERROR("New session without create command!\n");
> + r = -ENOENT;
> + }
> +
> +out:
> + if ((!r && destroyed) || (r && allocated)) {
> + /*
> + * IB contains a destroy msg or we have allocated an
> + * handle and got an error, anyway free the handle
> + */
> for (i = 0; i < RADEON_MAX_VCE_HANDLES; ++i)
> atomic_cmpxchg(&p->rdev->vce.handles[i], handle, 0);
> }
>
> - return 0;
> + return r;
> }
>
> /**
> --
> 1.9.1
>
More information about the dri-devel
mailing list