[Mesa-dev] [PATCH 02/27] i965/miptree: Remove redundant check for null texture

[Jason Ekstrand]

On Tue, Jan 17, 2017 at 12:21 AM, Pohjolainen, Topi <
topi.pohjolainen at gmail.com> wrote:

> On Mon, Jan 16, 2017 at 08:33:09AM -0800, Jason Ekstrand wrote:
> >    On Mon, Jan 16, 2017 at 1:13 AM, Topi Pohjolainen
> >    <[1]topi.pohjolainen at gmail.com> wrote:
> >
> >      There exact same check earlier in brw_miptree_layout() which
> >      intel_miptree_create_layout() in turn calls unconditionally.
> >      Signed-off-by: Topi Pohjolainen <[2]topi.pohjolainen at intel.com>
> >      ---
> >       src/mesa/drivers/dri/i965/intel_mipmap_tree.c | 7 +------
> >       1 file changed, 1 insertion(+), 6 deletions(-)
> >      diff --git a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> >      b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> >      index 25f8f39..9488bec 100644
> >      --- a/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> >      +++ b/src/mesa/drivers/dri/i965/intel_mipmap_tree.c
> >      @@ -628,13 +628,8 @@ miptree_create(struct brw_context *brw,
> >                                           first_level, last_level,
> >      width0,
> >                                           height0, depth0, num_samples,
> >                                           layout_flags);
> >      -   /*
> >      -    * pitch == 0 || height == 0  indicates the null texture
> >      -    */
> >      -   if (!mt || !mt->total_width || !mt->total_height) {
> >      -      intel_miptree_release(&mt);
> >      +   if (!mt)
> >             return NULL;
> >
> >    Ugh... Not quite.  More miptree nastiness!  Looking through the code,
> >    brw_miptree_layout does do this check and unrefs the miptree but has
> no
> >    way of indicating to higher levels that it has unref'd the miptree!
> In
> >    other words, if that ever happens, the aux_disable lines at the end of
> >    intel_miptree_create_layout will read/write freed memory and
> >    intel_miptree_create_layout will a valid-looking (but freed) pointer.
>
> Indeed, good catch!!
>
> I sent an additional patch that propagates the error. This patch should be
> safe on top of that.
>

Thanks!  1.5 and 2  are

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>


> >
> >      -   }
> >          if (mt->tiling == (I915_TILING_Y | I915_TILING_X))
> >             mt->tiling = I915_TILING_Y;
> >      --
> >      2.5.5
> >      _______________________________________________
> >      mesa-dev mailing list
> >      [3]mesa-dev at lists.freedesktop.org
> >      [4]https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >
> > References
> >
> >    1. mailto:topi.pohjolainen at gmail.com
> >    2. mailto:topi.pohjolainen at intel.com
> >    3. mailto:mesa-dev at lists.freedesktop.org
> >    4. https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170117/466e2a32/attachment-0001.html>