Skip to content

wlr_scene: Don't use after free on degenerate wlr_scene_output

When destroying the wlr_scene_output, there are three objects at play: the wlr_scene_output itself, its wlr_output_damage and of course the wlr_output.

While the wlr_output is being destroyed it will first call destroy listeners then move on to destroy the addons. The wlr_output_damage is listening on destroy events in this first destroy signal. Concurrently the wlr_scene_output is waiting for a destroy event to come in through the addon destroy event. This means that between these two time frames the wlr_scene_output will have a freed damage object but the wlr_scene_outputs itself will not be freed.

Consider this: I have a wlr_output and I add my own destroy listener to it

wlr_output:
    - destroy events
        - my innocent little destroy listener in my compositor
    - addons

Now suppose I create a wlr_scene_output:

wlr_output:
    - destroy events
        - wlr_output_damage listener
        - my innocent little destroy listener in my compositor
    - addons
        - wlr_scene_output addon

Now my innocent destroy listener is sandwiched between the output damage being destroyed and the scene output actual getting destroyed through its addon. If my innocent destroy function won't be so innocent anymore if I decide to do anything with the wlr_scene that tries to access the output damage, say destroy a scene node: you'll get UAF and SIGSEGV.

To fix destroy the wlr_scene_output when the damage object gets destroyed or the addon is finished; whichever comes first.

* thread #1, name = 'sway', stop reason = signal SIGSEGV: invalid address (fault address: 0x5550030abe93)
    frame #0: 0x00007ffff78d8ae5 libwlroots.so.11`wlr_output_transformed_resolution(output=0x00005550030abdc3, width=0x00007fffffffdd98, height=0x00007fffffffdd9c) at output.c:462:12
   459
   460 	void wlr_output_transformed_resolution(struct wlr_output *output,
   461 			int *width, int *height) {
-> 462 		if (output->transform % 2 == 0) {
   463 			*width = output->width;
   464 			*height = output->height;
   465 		} else {
(lldb) up
error: sway {0x000357d7}: DIE has DW_AT_ranges(DW_FORM_sec_offset 0x67) attribute, but range extraction failed (invalid range list offset 0x67), please file a bug and attach the file at the start of this error message
error: sway {0x000357d7}: DIE has DW_AT_ranges(DW_FORM_sec_offset 0x67) attribute, but range extraction failed (invalid range list offset 0x67), please file a bug and attach the file at the start of this error message
frame #1: 0x00007ffff790503d libwlroots.so.11`wlr_output_damage_add(output_damage=0x0000555555f53650, damage=0x00007fffffffdde0) at wlr_output_damage.c:188:2
   185 	void wlr_output_damage_add(struct wlr_output_damage *output_damage,
   186 			pixman_region32_t *damage) {
   187 		int width, height;
-> 188 		wlr_output_transformed_resolution(output_damage->output, &width, &height);
   189
   190 		pixman_region32_t clipped_damage;
   191 		pixman_region32_init(&clipped_damage);
(lldb) up
frame #2: 0x00007ffff79051c1 libwlroots.so.11`wlr_output_damage_add_box(output_damage=0x0000555555f53650, box=0x00007fffffffde50) at wlr_output_damage.c:218:2
   215 		pixman_region32_t damage;
   216 		pixman_region32_init_rect(&damage,
   217 			box->x, box->y, box->width, box->height);
-> 218 		wlr_output_damage_add(output_damage, &damage);
   219 		pixman_region32_fini(&damage);
   220 	}
(lldb) up
frame #3: 0x00007ffff78dd186 libwlroots.so.11`_scene_node_damage_whole(node=0x00005555565c9bb0, scene=0x0000555555602e50, lx=0, ly=0) at wlr_scene.c:531:3
   528
   529 			scale_box(&box, scene_output->output->scale);
   530
-> 531 			wlr_output_damage_add_box(scene_output->damage, &box);
   532 		}
   533 	}
   534
(lldb) up
frame #4: 0x00007ffff78dd0da libwlroots.so.11`_scene_node_damage_whole(node=0x00005555565d4e20, scene=0x0000555555602e50, lx=0, ly=0) at wlr_scene.c:512:4
   509 			struct wlr_scene_tree *scene_tree = scene_tree_from_node(node);
   510 			struct wlr_scene_node *child;
   511 			wl_list_for_each(child, &scene_tree->children, link) {
-> 512 				_scene_node_damage_whole(child, scene,
   513 					lx + child->x, ly + child->y);
   514 			}
   515 		}
(lldb) up
frame #5: 0x00007ffff78dd0da libwlroots.so.11`_scene_node_damage_whole(node=0x00005555566152b0, scene=0x0000555555602e50, lx=0, ly=0) at wlr_scene.c:512:4
   509 			struct wlr_scene_tree *scene_tree = scene_tree_from_node(node);
   510 			struct wlr_scene_node *child;
   511 			wl_list_for_each(child, &scene_tree->children, link) {
-> 512 				_scene_node_damage_whole(child, scene,
   513 					lx + child->x, ly + child->y);
   514 			}
   515 		}
(lldb) up
frame #6: 0x00007ffff78dd22c libwlroots.so.11`scene_node_damage_whole(node=0x00005555566152b0) at wlr_scene.c:546:2
   543 			return;
   544 		}
   545
-> 546 		_scene_node_damage_whole(node, scene, lx, ly);
   547 	}
   548
   549 	void wlr_scene_node_set_enabled(struct wlr_scene_node *node, bool enabled) {
(lldb) up
frame #7: 0x00007ffff78dbdd0 libwlroots.so.11`wlr_scene_node_destroy(node=0x00005555566152b0) at wlr_scene.c:78:2
   75  			return;
   76  		}
   77
-> 78  		scene_node_damage_whole(node);
   79
   80  		// We want to call the destroy listeners before we do anything else
   81  		// incase the destroy signal would like to remove children before they
(lldb) up
frame #8: 0x0000555555577d47 sway`handle_output_destroy(listener=0x00005555565d6670, data=0x0000555555f51620) at layer_shell.c:127:2
   124 			}
   125 		}
   126
-> 127 		wlr_scene_node_destroy(&layer->scene->tree->node);
   128 		layer->output = NULL;
   129 	}
   130
Edited by Alexander Orzechowski

Merge request reports