Re: [PATCH] drm/i915: Fix vmap_batch page iterator overrun

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 03/13/2015 01:21 PM, Mika Kuoppala wrote:
> vmap_batch() calculates amount of needed pages for the mapping
> we are going to create. And it uses this page count as an
> argument for the for_each_sg_pages() macro. The macro takes the number
> of sg list entities as an argument, not the page count. So we ended
> up iterating through all the pages on the mapped object, corrupting
> memory past the smaller pages[] array.
>
> Fix this by bailing out when we have enough pages.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_cmd_parser.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
> index 9a6da35..61ae8ff 100644
> --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> @@ -836,8 +836,11 @@ static u32 *vmap_batch(struct drm_i915_gem_object *obj,
>   	}
>
>   	i = 0;
> -	for_each_sg_page(obj->pages->sgl, &sg_iter, npages, first_page)
> +	for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, first_page) {
>   		pages[i++] = sg_page_iter_page(&sg_iter);
> +		if (i == npages)
> +			break;
> +	}

Are you sure this manual check is needed now that you fixed 
for_each_sg_page?

Pages array looks pessimistically big enough so I don't see that memory 
was getting overwritten. It looks more like our sg table was not 
properly terminated which made for_each_sg_page go into random memory 
and return random page pointers.

Regards,

Tvrtko




_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx