From e11d4dc0de2d6688bf55fb44ca8b1b5d6f80ddac Mon Sep 17 00:00:00 2001 From: Isaac Freund Date: Tue, 28 Feb 2023 18:19:37 +0100 Subject: [PATCH] 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, };