[xserver] xwayland: remove dirty window unconditionally on unrealize

Submitted by Olivier Fourdan on Jan. 24, 2018, 4:45 p.m.

Details

Message ID 20180124164537.27330-1-ofourdan@redhat.com
State Accepted
Commit 3362422e8413dd9f231cfac50ce0a0862525b1bf
Headers show
Series "xwayland: avoid a crash with empty window pixmaps" ( rev: 2 ) in X.org (DEPRECATED - USE GITLAB)

Not browsing as part of any series.

Commit Message

Olivier Fourdan Jan. 24, 2018, 4:45 p.m.
This is a rare occurrence of a crash in Xwayland for which I don't have
the reproducing steps, just a core file.

The backtrace looks as follow:

  #0  raise () from /usr/lib64/libc.so.6
  #1  abort () from /usr/lib64/libc.so.6
  #2  OsAbort () at utils.c:1361
  #3  AbortServer () at log.c:877
  #4  FatalError () at log.c:1015
  #5  OsSigHandler () at osinit.c:154
  #6  <signal handler called>
  #7  xwl_glamor_pixmap_get_wl_buffer () at xwayland-glamor.c:162
  #8  xwl_screen_post_damage () at xwayland.c:514
  #9  block_handler () at xwayland.c:665
  #10 BlockHandler () at dixutils.c:388
  #11 WaitForSomething () at WaitFor.c:219
  #12 Dispatch () at dispatch.c:422
  #13 dix_main () at main.c:287

The crash is caused by dereferencing “xwl_pixmap->buffer” in
xwl_glamor_pixmap_get_wl_buffer() because “xwl_pixmap” is NULL.

Reason for this is because the corresponding pixmap is from the root
window and xwayland is rootless by default.

This can happen if the window was mapped, redirected, damaged and
unredirected immediately, before the damage is processed by Xwayland.

Make sure to remove the dirty window from the damage list on unrealize
to prevent this from happening.

Credit goes to Adam Jackson <ajax@nwnk.net> and Daniel Stone
<daniel@fooishbar.org> for finding the root cause the issue.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
---
 hw/xwayland/xwayland.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

Patch hide | download patch | download mbox

diff --git a/hw/xwayland/xwayland.c b/hw/xwayland/xwayland.c
index ab7cee545..88d31f80b 100644
--- a/hw/xwayland/xwayland.c
+++ b/hw/xwayland/xwayland.c
@@ -587,8 +587,7 @@  xwl_unrealize_window(WindowPtr window)
     }
 
     wl_surface_destroy(xwl_window->surface);
-    if (RegionNotEmpty(DamageRegion(xwl_window->damage)))
-        xorg_list_del(&xwl_window->link_damage);
+    xorg_list_del(&xwl_window->link_damage);
     DamageUnregister(xwl_window->damage);
     DamageDestroy(xwl_window->damage);
     if (xwl_window->frame_callback)

Comments

Hi Olivier,

On 24 January 2018 at 16:45, Olivier Fourdan <ofourdan@redhat.com> wrote:
> This is a rare occurrence of a crash in Xwayland for which I don't have
> the reproducing steps, just a core file.
>
> The backtrace looks as follow:
>   [...]
>
> The crash is caused by dereferencing “xwl_pixmap->buffer” in
> xwl_glamor_pixmap_get_wl_buffer() because “xwl_pixmap” is NULL.
>
> Reason for this is because the corresponding pixmap is from the root
> window and xwayland is rootless by default.
>
> This can happen if the window was mapped, redirected, damaged and
> unredirected immediately, before the damage is processed by Xwayland.
>
> Make sure to remove the dirty window from the damage list on unrealize
> to prevent this from happening.
>
> Credit goes to Adam Jackson <ajax@nwnk.net> and Daniel Stone
> <daniel@fooishbar.org> for finding the root cause the issue.
>
> Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>

This is also:
Reviewed-by: Daniel Stone <daniels@collabora.com>

Cheers,
Daniel
On Thu, 2018-01-25 at 12:52 +0000, Daniel Stone wrote:
> Hi Olivier,
> 
> On 24 January 2018 at 16:45, Olivier Fourdan <ofourdan@redhat.com> wrote:
> > Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
> 
> This is also:
> Reviewed-by: Daniel Stone <daniels@collabora.com>

Merged both:

remote: I: patch #200439 updated using rev 3362422e8413dd9f231cfac50ce0a0862525b1bf.
remote: I: patch #200643 updated using rev fc8b7d05e74a6351df56ad8b17216aeb0dcc016b.
remote: I: 2 patch(es) updated to state Accepted.
To ssh://git.freedesktop.org/git/xorg/xserver
   29a5423abd..fc8b7d05e7  master -> master

- ajax