<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Tue, Jan 17, 2017 at 12:21 AM, Pohjolainen, Topi <span dir="ltr"><<a href="mailto:topi.pohjolainen@gmail.com" target="_blank">topi.pohjolainen@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On Mon, Jan 16, 2017 at 08:33:09AM -0800, Jason Ekstrand wrote:<br>
>    On Mon, Jan 16, 2017 at 1:13 AM, Topi Pohjolainen<br>
</span><span class="">>    <[1]<a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.com</a><wbr>> wrote:<br>
><br>
>      There exact same check earlier in brw_miptree_layout() which<br>
>      intel_miptree_create_layout() in turn calls unconditionally.<br>
</span>>      Signed-off-by: Topi Pohjolainen <[2]<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.com</a><wbr>><br>
<div><div class="h5">>      ---<br>
>       src/mesa/drivers/dri/i965/<wbr>intel_mipmap_tree.c | 7 +------<br>
>       1 file changed, 1 insertion(+), 6 deletions(-)<br>
>      diff --git a/src/mesa/drivers/dri/i965/<wbr>intel_mipmap_tree.c<br>
>      b/src/mesa/drivers/dri/i965/<wbr>intel_mipmap_tree.c<br>
>      index 25f8f39..9488bec 100644<br>
>      --- a/src/mesa/drivers/dri/i965/<wbr>intel_mipmap_tree.c<br>
>      +++ b/src/mesa/drivers/dri/i965/<wbr>intel_mipmap_tree.c<br>
>      @@ -628,13 +628,8 @@ miptree_create(struct brw_context *brw,<br>
>                                           first_level, last_level,<br>
>      width0,<br>
>                                           height0, depth0, num_samples,<br>
>                                           layout_flags);<br>
>      -   /*<br>
>      -    * pitch == 0 || height == 0  indicates the null texture<br>
>      -    */<br>
>      -   if (!mt || !mt->total_width || !mt->total_height) {<br>
>      -      intel_miptree_release(&mt);<br>
>      +   if (!mt)<br>
>             return NULL;<br>
><br>
>    Ugh... Not quite.  More miptree nastiness!  Looking through the code,<br>
>    brw_miptree_layout does do this check and unrefs the miptree but has no<br>
>    way of indicating to higher levels that it has unref'd the miptree!  In<br>
>    other words, if that ever happens, the aux_disable lines at the end of<br>
>    intel_miptree_create_layout will read/write freed memory and<br>
>    intel_miptree_create_layout will a valid-looking (but freed) pointer.<br>
<br>
</div></div>Indeed, good catch!!<br>
<br>
I sent an additional patch that propagates the error. This patch should be<br>
safe on top of that.<span class=""><br></span></blockquote><div><br></div><div>Thanks!  1.5 and 2  are<br><br></div><div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">
><br>
>      -   }<br>
>          if (mt->tiling == (I915_TILING_Y | I915_TILING_X))<br>
>             mt->tiling = I915_TILING_Y;<br>
>      --<br>
>      2.5.5<br>
>      ______________________________<wbr>_________________<br>
>      mesa-dev mailing list<br>
</span>>      [3]<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.<wbr>org</a><br>
>      [4]<a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.<wbr>org/mailman/listinfo/mesa-dev</a><br>
><br>
> References<br>
><br>
>    1. mailto:<a href="mailto:topi.pohjolainen@gmail.com">topi.pohjolainen@gmail.<wbr>com</a><br>
>    2. mailto:<a href="mailto:topi.pohjolainen@intel.com">topi.pohjolainen@intel.<wbr>com</a><br>
>    3. mailto:<a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.<wbr>freedesktop.org</a><br>
>    4. <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev" rel="noreferrer" target="_blank">https://lists.freedesktop.org/<wbr>mailman/listinfo/mesa-dev</a><br>
</blockquote></div><br></div></div>