Compare revisions

Changes are shown as if the source revision was being merged into the target revision. Learn more about comparing revisions.

Source

Target

Show changes

Only incoming changes from source

Include changes to target since source was created

Commits on Source (2)

xwayland/present: Fix use-after-free in xwl_unrealize_window() · 42113ab2

Olivier Fourdan authored 3 years ago and

Olivier Fourdan committed 3 years ago


When a window is unrealized, Xwayland would destroy the Wayland surface
prior to unrealizing the present window.

xwl_present_flip() will then do a wl_surface_commit() of that surface,
hence causing a use-after-free:

 Invalid read of size 8
    at 0x49F7FD4: wl_proxy_marshal_array_flags (wayland-client.c:852)
    by 0x49F823A: wl_proxy_marshal_flags (wayland-client.c:784)
    by 0x42B877: wl_surface_commit (wayland-client-protocol.h:3914)
    by 0x42CAA7: xwl_present_flip (xwayland-present.c:717)
    by 0x42CD0E: xwl_present_execute (xwayland-present.c:783)
    by 0x42C26D: xwl_present_msc_bump (xwayland-present.c:416)
    by 0x42C2D1: xwl_present_timer_callback (xwayland-present.c:433)
    by 0x42BAC4: xwl_present_reset_timer (xwayland-present.c:149)
    by 0x42D1F8: xwl_present_unrealize_window (xwayland-present.c:945)
    by 0x4230E2: xwl_unrealize_window (xwayland-window.c:616)
    by 0x4FCDD8: compUnrealizeWindow (compwindow.c:292)
    by 0x4F3F5C: UnrealizeTree (window.c:2805)
  Address 0x1390b8d8 is 24 bytes inside a block of size 80 free'd
    at 0x48470E4: free (vg_replace_malloc.c:872)
    by 0x49F8029: wl_proxy_destroy_caller_locks (wayland-client.c:523)
    by 0x49F8029: wl_proxy_marshal_array_flags (wayland-client.c:861)
    by 0x49F823A: wl_proxy_marshal_flags (wayland-client.c:784)
    by 0x421984: wl_surface_destroy (wayland-client-protocol.h:3672)
    by 0x423052: xwl_unrealize_window (xwayland-window.c:599)
    by 0x4FCDD8: compUnrealizeWindow (compwindow.c:292)
    by 0x4F3F5C: UnrealizeTree (window.c:2805)
    by 0x4F424B: UnmapWindow (window.c:2863)
    by 0x4EF58C: DeleteWindow (window.c:1075)
    by 0x4E24B3: doFreeResource (resource.c:885)
    by 0x4E2ED7: FreeClientResources (resource.c:1151)
    by 0x4ACBA4: CloseDownClient (dispatch.c:3546)
  Block was alloc'd at
    at 0x4849464: calloc (vg_replace_malloc.c:1328)
    by 0x49F7F29: zalloc (wayland-private.h:233)
    by 0x49F7F29: proxy_create (wayland-client.c:422)
    by 0x49F7F29: create_outgoing_proxy (wayland-client.c:664)
    by 0x49F7F29: wl_proxy_marshal_array_flags (wayland-client.c:831)
    by 0x49F823A: wl_proxy_marshal_flags (wayland-client.c:784)
    by 0x4218CA: wl_compositor_create_surface (wayland-client-protocol.h:1291)
    by 0x422A0D: ensure_surface_for_window (xwayland-window.c:445)
    by 0x4231E8: xwl_window_set_window_pixmap (xwayland-window.c:647)
    by 0x5232D6: damageSetWindowPixmap (damage.c:1565)
    by 0x4FC7BC: compSetPixmapVisitWindow (compwindow.c:129)
    by 0x4EDB3F: TraverseTree (window.c:441)
    by 0x4FC851: compSetPixmap (compwindow.c:151)
    by 0x4F8C1A: compAllocPixmap (compalloc.c:616)
    by 0x4FC938: compCheckRedirect (compwindow.c:174)

To avoid that, call xwl_present_unrealize_window() before destroying the
Wayland surface.

Signed-off-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>

42113ab2

xwayland: Clear timer_armed in xwl_present_unrealize_window · 102764b6

Michel Dänzer authored 3 years ago and

Olivier Fourdan committed 3 years ago


Without this, xwl_present_reset_timer would call
xwl_present_timer_callback if the timer was originally armed over a
second ago. xwl_present_timer_callback would call xwl_present_msc_bump,
which could end up hooking up the window to
xwl_window->frame_callback_list again. This would lead to use-after-free
in xwl_present_cleanup:

  Invalid write of size 8
    at 0x42B65C: __xorg_list_del (list.h:183)
    by 0x42B693: xorg_list_del (list.h:204)
    by 0x42C041: xwl_present_cleanup (xwayland-present.c:354)
    by 0x423669: xwl_destroy_window (xwayland-window.c:770)
    by 0x4FDDC5: compDestroyWindow (compwindow.c:620)
    by 0x5233FB: damageDestroyWindow (damage.c:1590)
    by 0x501C5F: DbeDestroyWindow (dbe.c:1326)
    by 0x4EF35B: FreeWindowResources (window.c:1018)
    by 0x4EF687: DeleteWindow (window.c:1086)
    by 0x4E24B3: doFreeResource (resource.c:885)
    by 0x4E2ED7: FreeClientResources (resource.c:1151)
    by 0x4ACBA4: CloseDownClient (dispatch.c:3546)
  Address 0x12f44980 is 144 bytes inside a block of size 160 free'd
    at 0x48470E4: free (vg_replace_malloc.c:872)
    by 0x423115: xwl_unrealize_window (xwayland-window.c:621)
    by 0x4FCDD8: compUnrealizeWindow (compwindow.c:292)
    by 0x4F3F5C: UnrealizeTree (window.c:2805)
    by 0x4F424B: UnmapWindow (window.c:2863)
    by 0x4EF58C: DeleteWindow (window.c:1075)
    by 0x4E24B3: doFreeResource (resource.c:885)
    by 0x4E2ED7: FreeClientResources (resource.c:1151)
    by 0x4ACBA4: CloseDownClient (dispatch.c:3546)
    by 0x5E27EE: ClientReady (connection.c:599)
    by 0x5E6CB7: ospoll_wait (ospoll.c:657)
    by 0x5DE6CD: WaitForSomething (WaitFor.c:208)
  Block was alloc'd at
    at 0x4849464: calloc (vg_replace_malloc.c:1328)
    by 0x4229CE: ensure_surface_for_window (xwayland-window.c:439)
    by 0x4231E8: xwl_window_set_window_pixmap (xwayland-window.c:647)
    by 0x5232D6: damageSetWindowPixmap (damage.c:1565)
    by 0x4FC7BC: compSetPixmapVisitWindow (compwindow.c:129)
    by 0x4EDB3F: TraverseTree (window.c:441)
    by 0x4FC851: compSetPixmap (compwindow.c:151)
    by 0x4F8C1A: compAllocPixmap (compalloc.c:616)
    by 0x4FC938: compCheckRedirect (compwindow.c:174)
    by 0x4FCD1D: compRealizeWindow (compwindow.c:274)
    by 0x4F36EC: RealizeTree (window.c:2606)
    by 0x4F39F5: MapWindow (window.c:2683)

Fixes: 288ec0e0 ("xwayland/present: Run fallback timer callback after more than a second")
Tested-by: Olivier Fourdan <ofourdan@redhat.com>
Reviewed-by: Olivier Fourdan <ofourdan@redhat.com>

102764b6

Hide whitespace changes

Inline Side-by-side

Showing with 13 additions and 10 deletions

View file @ 102764b6

...	@@ -942,6 +942,9 @@ xwl_present_unrealize_window(struct xwl_present_window *xwl_present_window)	...	@@ -942,6 +942,9 @@ xwl_present_unrealize_window(struct xwl_present_window *xwl_present_window)
	* the frame timer interval.		* the frame timer interval.
	*/		*/
	xorg_list_del(&xwl_present_window->frame_callback_list);		xorg_list_del(&xwl_present_window->frame_callback_list);

			/* Make sure the timer callback doesn't get called */
			xwl_present_window->timer_armed = 0;
	xwl_present_reset_timer(xwl_present_window);		xwl_present_reset_timer(xwl_present_window);
	}		}

...		...

View file @ 102764b6

...	@@ -605,16 +605,6 @@ xwl_unrealize_window(WindowPtr window)	...	@@ -605,16 +605,6 @@ xwl_unrealize_window(WindowPtr window)
	if (xwl_window_has_viewport_enabled(xwl_window))		if (xwl_window_has_viewport_enabled(xwl_window))
	xwl_window_disable_viewport(xwl_window);		xwl_window_disable_viewport(xwl_window);

	wl_surface_destroy(xwl_window->surface);
	xorg_list_del(&xwl_window->link_damage);
	xorg_list_del(&xwl_window->link_window);
	unregister_damage(window);

	xwl_window_buffers_dispose(xwl_window);

	if (xwl_window->frame_callback)
	wl_callback_destroy(xwl_window->frame_callback);

	#ifdef GLAMOR_HAS_GBM		#ifdef GLAMOR_HAS_GBM
	if (xwl_screen->present) {		if (xwl_screen->present) {
	struct xwl_present_window xwl_present_window, tmp;		struct xwl_present_window xwl_present_window, tmp;
...	@@ -627,6 +617,16 @@ xwl_unrealize_window(WindowPtr window)	...	@@ -627,6 +617,16 @@ xwl_unrealize_window(WindowPtr window)
	}		}
	#endif		#endif

			wl_surface_destroy(xwl_window->surface);
			xorg_list_del(&xwl_window->link_damage);
			xorg_list_del(&xwl_window->link_window);
			unregister_damage(window);

			xwl_window_buffers_dispose(xwl_window);

			if (xwl_window->frame_callback)
			wl_callback_destroy(xwl_window->frame_callback);

	free(xwl_window);		free(xwl_window);
	dixSetPrivate(&window->devPrivates, &xwl_window_private_key, NULL);		dixSetPrivate(&window->devPrivates, &xwl_window_private_key, NULL);

...		...

Admin message

Admin message

Compare revisions

Source

Target