git.lucas.co / cce-compositor
Wayland compositor (wlroots)
git clone https://git.lucas.co/cce-compositor.git

commit25d9b0d15f43ea12335c71e3fd442b8320ec3fea
parente0245ff461
authorIsaac Freund <[email protected]>
date2023-02-28 18:19
LayerSurface: fix use-after-free on destroy

The scene_layer_surface may be destroyed before handleDestroy is called,
which means we can't rely on it to access the wlr_layer_surface in
destroyPopups().

 river/Cursor.zig       |  2 +-
 river/LayerSurface.zig | 14 ++++++++------
 river/Root.zig         |  2 +-
 river/Seat.zig         |  4 ++--
 4 files changed, 12 insertions(+), 10 deletions(-)

diff --git a/river/Cursor.zig b/river/Cursor.zig
index edc3351..84cda6a 100644
--- a/river/Cursor.zig
+++ b/river/Cursor.zig
@@ -346,7 +346,7 @@ fn updateKeyboardFocus(self: Self, result: Root.AtResult) void {
             self.seat.focusOutput(layer_surface.output);
             // If a keyboard inteactive layer surface has been clicked on,
             // give it keyboard focus.
-            if (layer_surface.scene_layer_surface.layer_surface.current.keyboard_interactive != .none) {
+            if (layer_surface.wlr_layer_surface.current.keyboard_interactive != .none) {
                 self.seat.setFocusRaw(.{ .layer = layer_surface });
             } else {
                 self.seat.focus(null);
diff --git a/river/LayerSurface.zig b/river/LayerSurface.zig
index 8ad75b5..b3cdec3 100644
--- a/river/LayerSurface.zig
+++ b/river/LayerSurface.zig
@@ -32,6 +32,7 @@ const XdgPopup = @import("XdgPopup.zig");
 const log = std.log.scoped(.layer_shell);
 
 output: *Output,
+wlr_layer_surface: *wlr.LayerSurfaceV1,
 scene_layer_surface: *wlr.SceneLayerSurfaceV1,
 popup_tree: *wlr.SceneTree,
 
@@ -50,6 +51,7 @@ pub fn create(wlr_layer_surface: *wlr.LayerSurfaceV1) error{OutOfMemory}!void {
 
     layer_surface.* = .{
         .output = output,
+        .wlr_layer_surface = wlr_layer_surface,
         .scene_layer_surface = try layer_tree.createSceneLayerSurfaceV1(wlr_layer_surface),
         .popup_tree = try output.layers.popups.createSceneTree(),
     };
@@ -71,14 +73,14 @@ pub fn create(wlr_layer_surface: *wlr.LayerSurfaceV1) error{OutOfMemory}!void {
 }
 
 pub fn destroyPopups(layer_surface: *LayerSurface) void {
-    var it = layer_surface.scene_layer_surface.layer_surface.popups.safeIterator(.forward);
+    var it = layer_surface.wlr_layer_surface.popups.safeIterator(.forward);
     while (it.next()) |wlr_xdg_popup| wlr_xdg_popup.destroy();
 }
 
-fn handleDestroy(listener: *wl.Listener(*wlr.LayerSurfaceV1), wlr_layer_surface: *wlr.LayerSurfaceV1) void {
+fn handleDestroy(listener: *wl.Listener(*wlr.LayerSurfaceV1), _: *wlr.LayerSurfaceV1) void {
     const layer_surface = @fieldParentPtr(LayerSurface, "destroy", listener);
 
-    log.debug("layer surface '{s}' destroyed", .{wlr_layer_surface.namespace});
+    log.debug("layer surface '{s}' destroyed", .{layer_surface.wlr_layer_surface.namespace});
 
     layer_surface.destroy.link.remove();
     layer_surface.map.link.remove();
@@ -112,7 +114,7 @@ fn handleUnmap(listener: *wl.Listener(*wlr.LayerSurfaceV1), wlr_layer_surface: *
 
 fn handleCommit(listener: *wl.Listener(*wlr.Surface), _: *wlr.Surface) void {
     const layer_surface = @fieldParentPtr(LayerSurface, "commit", listener);
-    const wlr_layer_surface = layer_surface.scene_layer_surface.layer_surface;
+    const wlr_layer_surface = layer_surface.wlr_layer_surface;
 
     assert(wlr_layer_surface.output != null);
 
@@ -145,7 +147,7 @@ fn handleKeyboardInteractiveExclusive(output: *Output) void {
             assert(node.type == .tree);
             if (@intToPtr(?*SceneNodeData, node.data)) |node_data| {
                 const layer_surface = node_data.data.layer_surface;
-                const wlr_layer_surface = layer_surface.scene_layer_surface.layer_surface;
+                const wlr_layer_surface = layer_surface.wlr_layer_surface;
                 if (wlr_layer_surface.mapped and
                     wlr_layer_surface.current.keyboard_interactive == .exclusive)
                 {
@@ -167,7 +169,7 @@ fn handleKeyboardInteractiveExclusive(output: *Output) void {
             // seats.
             seat.setFocusRaw(.{ .layer = to_focus });
         } else if (seat.focused == .layer) {
-            const current_focus = seat.focused.layer.scene_layer_surface.layer_surface;
+            const current_focus = seat.focused.layer.wlr_layer_surface;
             // If the seat is currently focusing an unmapped layer surface or one
             // without keyboard interactivity, stop focusing that layer surface.
             if (!current_focus.mapped or current_focus.current.keyboard_interactive == .none) {
diff --git a/river/Root.zig b/river/Root.zig
index 747f9e6..3a892bd 100644
--- a/river/Root.zig
+++ b/river/Root.zig
@@ -283,7 +283,7 @@ pub fn removeOutput(root: *Self, output: *Output) void {
         while (it.next()) |scene_node| {
             assert(scene_node.type == .tree);
             if (@intToPtr(?*SceneNodeData, scene_node.data)) |node_data| {
-                node_data.data.layer_surface.scene_layer_surface.layer_surface.destroy();
+                node_data.data.layer_surface.wlr_layer_surface.destroy();
             }
         }
     }
diff --git a/river/Seat.zig b/river/Seat.zig
index 82b9cd8..d3c22d9 100644
--- a/river/Seat.zig
+++ b/river/Seat.zig
@@ -149,7 +149,7 @@ pub fn focus(self: *Self, _target: ?*View) void {
 
     // While a layer surface is exclusively focused, views may not receive focus
     if (self.focused == .layer) {
-        const wlr_layer_surface = self.focused.layer.scene_layer_surface.layer_surface;
+        const wlr_layer_surface = self.focused.layer.wlr_layer_surface;
         if (wlr_layer_surface.current.keyboard_interactive == .exclusive and
             (wlr_layer_surface.current.layer == .top or wlr_layer_surface.current.layer == .overlay))
         {
@@ -210,7 +210,7 @@ pub fn setFocusRaw(self: *Self, new_focus: FocusTarget) void {
     const target_surface = switch (new_focus) {
         .view => |target_view| target_view.rootSurface(),
         .xwayland_override_redirect => |target_or| target_or.xwayland_surface.surface,
-        .layer => |target_layer| target_layer.scene_layer_surface.layer_surface.surface,
+        .layer => |target_layer| target_layer.wlr_layer_surface.surface,
         .lock_surface => |lock_surface| lock_surface.wlr_lock_surface.surface,
         .none => null,
     };