[Mesa-dev] [PATCH RFC] st/mesa: check st_context in st_renderbuffer_delete()

[Marek Olšák]

This will leak the renderbuffer, but that's not the biggest problem.

In your bug report, you said that the renderbuffer was created by
intel_new_renderbuffer, but this change is for st/mesa. Something is
horribly wrong here. The intel driver should not ever end up in
st/mesa, because st/mesa is a different driver. What is going on here?

Marek

On Thu, Aug 2, 2018 at 8:29 AM, Olivier Fourdan <ofourdan at redhat.com> wrote:
> st_renderbuffer_delete() can segfault if we get a non-NULL context
> pointer but if the st_context is NULL:
>
>   Thread 1 "Xwayland" received signal SIGSEGV, Segmentation fault.
>   in st_renderbuffer_delete () at state_tracker/st_cb_fbo.c:241
>   241 pipe_surface_release(st->pipe, &strb->surface_srgb);
>   (gdb) bt
>   #0  st_renderbuffer_delete () at state_tracker/st_cb_fbo.c:241
>   #1  _mesa_reference_renderbuffer_ () at main/renderbuffer.c:212
>   #2  _mesa_reference_renderbuffer () at main/renderbuffer.h:72
>   #3  _mesa_free_framebuffer_data (0) at main/framebuffer.c:229
>   #4  _mesa_destroy_framebuffer () at main/framebuffer.c:207
>   #5  _mesa_reference_framebuffer_ () at main/framebuffer.c:265
>   #6  _mesa_reference_framebuffer () at main/framebuffer.h:63
>   #7  _mesa_free_context_data () at main/context.c:1326
>   #8  st_destroy_context () at state_tracker/st_context.c:653
>   #9  dri_destroy_context () at dri_context.c:239
>   #10 driDestroyContext () at dri_util.c:524
>   #11 __glXDRIcontextDestroy () at glxdriswrast.c:132
>   #12 __glXFreeContext () at glxext.c:190
>   #13 ContextGone () at glxext.c:82
>   #14 doFreeResource () at resource.c:880
>   #15 FreeResourceByType () at resource.c:941
>   #16 __glXDisp_DestroyContext () at glxcmds.c:437
>   #17 dispatch_DestroyContext () at vnd_dispatch_stubs.c:82
>   #18 Dispatch () at dispatch.c:478
>   #19 dix_main () at main.c:276
>   #20 __libc_start_main () from /lib64/libc.so.6
>   #21 _start () at glxcmds.c:125
>
>   (gdb) p st
>   $1 = (struct st_context *) 0x0
>
> Check for a non-NULL st_context pointer as well to avoid the crash.
>
> Bugzilla: https://bugzilla.redhat.com/1611140
> Signed-off-by: Olivier Fourdan <ofourdan at redhat.com>
> ---
>  Note: This fixes several bug reported downstream, like:
>   https://bugzilla.redhat.com/1611140
>   https://bugs.launchpad.net/ubuntu/+source/xorg-server/+bug/1762971
>   https://bugs.launchpad.net/ubuntu/+source/mesa/+bug/1754693
>   etc.
>  I don't know what this client actually does, but whatever it is it should
>  not crash Xwayland because of Mesa...
>  I tested this fix against the given reproducer (run snap on Wayland/Xwayland)
>  and it works.
>
>  src/mesa/state_tracker/st_cb_fbo.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/state_tracker/st_cb_fbo.c b/src/mesa/state_tracker/st_cb_fbo.c
> index 73414fdfa1..856d213b73 100644
> --- a/src/mesa/state_tracker/st_cb_fbo.c
> +++ b/src/mesa/state_tracker/st_cb_fbo.c
> @@ -238,8 +238,10 @@ st_renderbuffer_delete(struct gl_context *ctx, struct gl_renderbuffer *rb)
>     struct st_renderbuffer *strb = st_renderbuffer(rb);
>     if (ctx) {
>        struct st_context *st = st_context(ctx);
> -      pipe_surface_release(st->pipe, &strb->surface_srgb);
> -      pipe_surface_release(st->pipe, &strb->surface_linear);
> +      if (st) {
> +         pipe_surface_release(st->pipe, &strb->surface_srgb);
> +         pipe_surface_release(st->pipe, &strb->surface_linear);
> +      }
>        strb->surface = NULL;
>     }
>     pipe_resource_reference(&strb->texture, NULL);
> --
> 2.17.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev