[Intel-gfx] [PATCH] drm/i915: prevent out of range pt in the PDE macros (take 3)

[Chris Wilson]

On Fri, Oct 02, 2015 at 09:58:05AM +0200, Daniel Vetter wrote:
> On Thu, Oct 01, 2015 at 04:59:35PM +0100, Michel Thierry wrote:
> > We tried to fix this in commit fdc454c1484a ("drm/i915: Prevent out of
> > range pt in gen6_for_each_pde").
> > 
> > But the static analyzer still complains that, just before we break due
> > to "iter < I915_PDES", we do "pt = (pd)->page_table[iter]" with an
> > iter value that is bigger than I915_PDES. Of course, this isn't really
> > a problem since no one uses pt outside the macro. Still, every single
> > new usage of the macro will create a new issue for us to mark as a
> > false positive.
> > 
> > Also, Paulo re-started the discussion a while ago [1], but didn't end up
> > implemented.
> > 
> > In order to "solve" this "problem", this patch takes the ideas from
> > Chris and Dave, but that check would change the desired behavior of the
> > code, because the object (for example pdp->page_directory[iter]) can be
> > null during init/alloc, and C would take this as false, breaking the for
> > loop immediately.
> > 
> > This has been already verified with "static analysis tools".
> > 
> > [1]http://lists.freedesktop.org/archives/intel-gfx/2015-June/068548.html
> > 
> > Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Dave Gordon <david.s.gordon at intel.com>
> > Signed-off-by: Michel Thierry <michel.thierry at intel.com>
> 
> So maybe I'm dense and not seeing what's really going on, but the only
> thing we seem to be doing is create a pointer to arr[SIZE], i.e. a pointer
> to the element right after the last valid one. Pointer arithmetic and
> comparison are explicitly allowed by the C standard on such a pointer. The
> only thing not allowed is dereference it (which we don't seem to be doing
> here).

You're thinking of &(pd)->page_table[iter] (i.e. (pd)->page_table +
iter). There is an apparent dereference here of (pd)->page_table[ITER_SIZE].
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre