Bug 110601

Summary: i965 renderbuffer miptree creation failure not properly handled
Product: Mesa Reporter: Anssi Hannula <anssi.hannula>
Component: Drivers/DRI/i965Assignee: Intel 3D Bugs Mailing List <intel-3d-bugs>
Status: RESOLVED MOVED QA Contact: Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity: normal    
Priority: medium    
Version: git   
Hardware: All   
OS: Linux (All)   
i915 platform: i915 features:
Attachments: Fix for issue (1), singlesample_mt use after free
Hack for issue (2), irb->mt == NULL
Testcase Qt C++ program
Testcase apitrace

Description Anssi Hannula 2019-05-03 14:10:16 UTC
Created attachment 144142 [details]
Fix for issue (1), singlesample_mt use after free

In src/mesa/drivers/dri/i965/intel_mipmap_tree.c intel_update_winsys_renderbuffer_miptree() the intel_miptree_create_for_renderbuffer() call can fail and the function is prepared for it.

However, there are several issues with the error handling:

(1) On failure exit from intel_update_winsys_renderbuffer_miptree(), the singlesample_mt received via parameter remains assigned to irb->singlesample_mt. However, in both callsites (brw_context.c intel_process_dri2_buffer() and intel_update_image_buffer()) the miptree is released on failure. This will lead to use-after-free and undefined behavior, including crashes.

Either singlesample_mt should be NULLed or it should not be freed by caller.
I have no idea which one would be correct, but I've attached a patch that does the former that seems to fix the use-after-free issues.

(2) On failure exit from intel_update_winsys_renderbuffer_miptree(), irb->mt remains NULL. However, many functions seem to assume that irb->mt is non-null:

 (I) do_single_blorp_clear()
 (II) brw_postdraw_set_buffers_need_resolve()
 (III) update_renderbuffer_surfaces()
There are probably more, but those are the ones I saw crashes in with the test code.

An example callpath for (I) above is:

  => intel_prepare_render()
    => intel_update_renderbuffers()
      => intel_process_dri2_buffer()
        => intel_update_winsys_renderbuffer_miptree()
           => fails, irb->mt = NULL
  => brw_blorp_clear_color()
    => do_single_blorp_clear()
      => dereferences irb->mt, segfault

Attached is a hack patch that adds NULL checks to some places, but I imagine a better way would be to somehow remove the renderbuffer altogether or something to avoid having to add mt NULL checks everywhere...

I will open a separate issue for the failure in intel_miptree_create_for_renderbuffer() that triggers this, but I think the error handling should be fixed regardless of the failure reason - at least so that there are no crashes.

I've attached the test application I used as mesa-issue-qt3d.cc. With the patches I still don't see correct graphical output with it (I see the red line as expected but the picture flashes rapidly), but I guess it may be expected with the miptree creation failure - I do see correct output after making miptree creation work.
Also included is an apitrace and glxinfo with mesa master without any patches.

This was tested on a ValleyView Gen7 (8086:0f31) with kernel 3.10.35, git master mesa (2019-05-02). AFAICS the issues can happen with any HW/kernel as long as intel_update_winsys_renderbuffer_miptree() manages to fail.
Comment 1 Anssi Hannula 2019-05-03 14:10:48 UTC
Created attachment 144143 [details] [review]
Hack for issue (2), irb->mt == NULL
Comment 2 Anssi Hannula 2019-05-03 14:11:28 UTC
Created attachment 144144 [details]
Testcase Qt C++ program
Comment 3 Anssi Hannula 2019-05-03 14:12:19 UTC
Created attachment 144145 [details]
Testcase apitrace
Comment 4 Anssi Hannula 2019-05-03 14:12:36 UTC
Created attachment 144146 [details]
Comment 5 GitLab Migration User 2019-09-25 20:33:13 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/mesa/mesa/issues/1809.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.