Re: [PATCH 2/2] drm/i915: Avoid dereference past end of page array in gen8_ppgtt_insert_entries()

[Ben Widawsky <benjamin.widawsky@intel.com>]

On Tue, Dec 31, 2013 at 03:50:31PM +0000, Chris Wilson wrote:
> The bug from gen6_ppgtt_insert_entries() was replicated into
> gen8_ppgtt_insert_entries(). This applies the fix for the OOPS from the
> previous patch to the gen8 routine.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ben Widawsky <benjamin.widawsky@intel.com>

Note to Daniel: This is still broken (for gen8) in the 4GB GGTT series.
There, pdpe is the bad guy.

Both are:
Reviewed-by: Ben Widawsky <ben@bwidawsk.net>

> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 18 +++++++++---------
>  1 file changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 366ede6352db..2a26d739a962 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -299,23 +299,23 @@ static void gen8_ppgtt_insert_entries(struct i915_address_space *vm,
>  	unsigned act_pte = first_entry % GEN8_PTES_PER_PAGE;
>  	struct sg_page_iter sg_iter;
>  
> -	pt_vaddr = kmap_atomic(&ppgtt->gen8_pt_pages[act_pt]);
> +	pt_vaddr = NULL;
>  	for_each_sg_page(pages->sgl, &sg_iter, pages->nents, 0) {
> -		dma_addr_t page_addr;
> +		if (pt_vaddr == NULL)
> +			pt_vaddr = kmap_atomic(&ppgtt->gen8_pt_pages[act_pt]);
>  
> -		page_addr = sg_dma_address(sg_iter.sg) +
> -				(sg_iter.sg_pgoffset << PAGE_SHIFT);
> -		pt_vaddr[act_pte] = gen8_pte_encode(page_addr, cache_level,
> -						    true);
> +		pt_vaddr[act_pte] =
> +			gen8_pte_encode(sg_page_iter_dma_address(&sg_iter),
> +					cache_level, true);
>  		if (++act_pte == GEN8_PTES_PER_PAGE) {
>  			kunmap_atomic(pt_vaddr);
> +			pt_vaddr = NULL;
>  			act_pt++;
> -			pt_vaddr = kmap_atomic(&ppgtt->gen8_pt_pages[act_pt]);
>  			act_pte = 0;
> -
>  		}
>  	}
> -	kunmap_atomic(pt_vaddr);
> +	if (pt_vaddr)
> +		kunmap_atomic(pt_vaddr);
>  }
>  
>  static void gen8_ppgtt_cleanup(struct i915_address_space *vm)
> -- 
> 1.8.5.2
> 

-- 
Ben Widawsky, Intel Open Source Technology Center