Re: [PATCH] drm/i915: add sanity check for partial view creation

From: Chris Wilson <chris@chris-wilson.co.uk>
To: Matthew Auld <matthew.auld@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: add sanity check for partial view creation
Date: Fri, 4 Mar 2016 10:53:27 +0000	[thread overview]
Message-ID: <20160304105327.GX30782@nuc-i3427.alporthouse.com> (raw)
In-Reply-To: <1457086284-14054-1-git-send-email-matthew.auld@intel.com>

On Fri, Mar 04, 2016 at 10:11:24AM +0000, Matthew Auld wrote:
> When binding pages for a partial view we should check that the offset +
> size is valid relative to the size of the gem object.
> 
> v2: Don't use pages->nents to determine the page count (Tvrtko Ursulin)
> v3: Handle potential overflow (Chris Wilson)
> 
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 7b8de85..596692b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -3493,6 +3493,13 @@ intel_partial_pages(const struct i915_ggtt_view *view,
>  	struct sg_page_iter obj_sg_iter;
>  	int ret = -ENOMEM;
>  
> +	if (U64_MAX - view->params.partial.offset < view->params.partial.size)
> +		return ERR_PTR(-ERANGE);

Idiomatically is this how we test for offset+size overflows?

> +	if (view->params.partial.offset + view->params.partial.size >
> +	    obj->base.size >> PAGE_SHIFT)
> +		return ERR_PTR(-EINVAL);

This is still idiotic (placement, choice of runtime errors for a
programmer error). If this concerns you that, please look at the API,
and please review the outstanding patches.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx