Bug 110601 - i965 renderbuffer miptree creation failure not properly handled
Summary: i965 renderbuffer miptree creation failure not properly handled
Alias: None
Product: Mesa
Classification: Unclassified
Component: Drivers/DRI/i965 (show other bugs)
Version: git
Hardware: All Linux (All)
: medium normal
Assignee: Intel 3D Bugs Mailing List
QA Contact: Intel 3D Bugs Mailing List
Depends on:
Reported: 2019-05-03 14:10 UTC by Anssi Hannula
Modified: 2019-09-25 20:33 UTC (History)
0 users

See Also:
i915 platform:
i915 features:

Fix for issue (1), singlesample_mt use after free (1011 bytes, application/mbox)
2019-05-03 14:10 UTC, Anssi Hannula
Hack for issue (2), irb->mt == NULL (2.58 KB, patch)
2019-05-03 14:10 UTC, Anssi Hannula
Details | Splinter Review
Testcase Qt C++ program (4.24 KB, text/plain)
2019-05-03 14:11 UTC, Anssi Hannula
Testcase apitrace (68.55 KB, application/octet-stream)
2019-05-03 14:12 UTC, Anssi Hannula
glxinfo (20.34 KB, text/plain)
2019-05-03 14:12 UTC, Anssi Hannula

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.