<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=Windows-1252"> <style type="text/css" style="display:none;"></style> </head> <body dir="ltr"> <div id="divtagdefaultwrapper" style="font-size:12pt;color:#000000;font-family:Calibri,Helvetica,sans-serif;" dir="ltr"> Emil: Your alternative patch won't work because dri_make_current is not necessarily called with NULL after a buffer has been destroyed. The problematic sequence is a pattern we use in QtWayland: //create temporary context surface1 = eglCreateWindowSurface() <-- dri_drawable pointer is malloced eglMakeCurrent(surface1) <-- ctx->dPriv is set // ... (Get some information about available GL extensions etc) eglDestroySurface(surface1) <-- pointer is freed, ctx->dPriv is now dangling surface2 = eglCreateWindowSurface() <-- Creating a new surface. Sometimes it's address will be the same as the free'd pointer. eglMakeCurrent(surface2) <-- In dri_make_current, ctx->dPriv == driReadPriv may return true because the pointers may be equal => The drawable info is not updated. Width and height for the drawable is not set from the wl_egl_window on the first frame. Marek: How exactly does it crash? Are you sure firefox didn't previously access free'd memory through the dangling pointer and that it was just exposed now that the pointer is NULL? Johan </div> <hr style="display:inline-block;width:98%" tabindex="-1"> <div id="divRplyFwdMsg" dir="ltr">From: Marek Olšák <maraeo@gmail.com> Sent: Tuesday, April 24, 2018 6:08:16 AM To: Johan Helsing Cc: ML Mesa-dev; pekka.paalanen@collabora.co.uk; Daniel Stone; Charmaine Lee; Thomas Hellstrom; Michel Dänzer Subject: Re: [PATCH] st/dri: Fix dangling pointer to a destroyed dri_drawable <div> </div> </div> <div> <div dir="ltr"> <div>FYI, the commit was causing crashes of qtcreator and firefox, so I reverted it. </div> Marek </div> <div class="x_gmail_extra"> <div class="x_gmail_quote">On Fri, Apr 20, 2018 at 6:29 AM, Johan Klokkhammer Helsing <<a href="mailto:johan.helsing@qt.io" target="_blank">johan.helsing@qt.io</a>> wrote: <blockquote class="x_gmail_quote" style="margin:0 0 0 .8ex; border-left:1px #ccc solid; padding-left:1ex"> If an EGLSurface is created, made current and destroyed, and then a second EGLSurface is created. Then the second malloc in driCreateNewDrawable may return the same pointer address the first surface's drawable had. Consequently, when dri_make_current later tries to determine if it should update the texture_stamp it compares the surface's drawable pointer against the drawable in the last call to dri_make_current and assumes it's the same surface (which it isn't). When texture_stamp is left unset, then dri_st_framebuffer_validate thinks it has already called update_drawable_info for that drawable, leaving it unvalidated and this is when bad things starts to happen. In my case it manifested itself by the width and height of the surface being unset. This is fixed this by setting the pointer to NULL before freeing the surface. Bugzilla: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=106126" rel="noreferrer" target="_blank"> https://bugs.freedesktop.org/show_bug.cgi?id=106126</a> Signed-off-by: Johan Klokkhammer Helsing <<a href="mailto:johan.helsing@qt.io">johan.helsing@qt.io</a>> --- src/gallium/state_trackers/dri/dri_drawable.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/gallium/state_trackers/dri/dri_drawable.c b/src/gallium/state_trackers/dri/dri_drawable.c index e5a7537e47..02328acd98 100644 --- a/src/gallium/state_trackers/dri/dri_drawable.c +++ b/src/gallium/state_trackers/dri/dri_drawable.c @@ -185,6 +185,7 @@ fail: void dri_destroy_buffer(__DRIdrawable * dPriv) { + struct dri_context *ctx = dri_context(dPriv->driContextPriv); struct dri_drawable *drawable = dri_drawable(dPriv); struct dri_screen *screen = drawable->screen; struct st_api *stapi = screen->st_api; @@ -202,6 +203,9 @@ dri_destroy_buffer(__DRIdrawable * dPriv) /* Notify the st manager that this drawable is no longer valid */ stapi->destroy_drawable(stapi, &drawable->base); + if (ctx && ctx->dPriv == dPriv) + ctx->dPriv = NULL; + FREE(drawable); } -- 2.17.0 </blockquote> </div> </div> </div> </body> </html>