From 0a5dddacfde337e10943e95b0d5aa5602d3c4e0c Mon Sep 17 00:00:00 2001 From: galister <22305755+galister@users.noreply.github.com> Date: Fri, 21 Jun 2024 09:13:51 +0900 Subject: [PATCH] fix crash when handling events with stale HopSlotMap key --- src/server/dispatch.rs | 8 +++-- src/server/event.rs | 59 +++++++++++++++++++++++------- src/server/mod.rs | 81 ++++++++++++++++++++++++++++-------------- 3 files changed, 107 insertions(+), 41 deletions(-) diff --git a/src/server/dispatch.rs b/src/server/dispatch.rs index 5999c13..e68988f 100644 --- a/src/server/dispatch.rs +++ b/src/server/dispatch.rs @@ -328,7 +328,7 @@ impl Dispatch for ServerState { hotspot_y, surface, } => { - let c_surface = surface.map(|s| state.get_client_surface_from_server(s)); + let c_surface = surface.and_then(|s| state.get_client_surface_from_server(s)); c_pointer.set_cursor(serial, c_surface, hotspot_x, hotspot_y); } Request::::Release => { @@ -786,8 +786,10 @@ impl ) { use s_vp::wp_viewporter; match request { - wp_viewporter::Request::GetViewport { id, surface } => { - let c_surface = state.get_client_surface_from_server(surface); + wp_viewporter::Request::GetViewport { id, surface } => 'get_viewport: { + let Some(c_surface) = state.get_client_surface_from_server(surface) else { + break 'get_viewport; + }; let c_viewport = client.get_viewport(c_surface, &state.qh, ()); data_init.init(id, c_viewport); } diff --git a/src/server/event.rs b/src/server/event.rs index 28b2d43..032df34 100644 --- a/src/server/event.rs +++ b/src/server/event.rs @@ -149,8 +149,18 @@ impl SurfaceData { let surface = &self.server; simple_event_shunt! { surface, event: client::wl_surface::Event => [ - Enter { |output| &state.get_object_from_client_object::(&output).server }, - Leave { |output| &state.get_object_from_client_object::(&output).server }, + Enter { |output| { + let Some(object) = &state.get_object_from_client_object::(&output) else { + return; + }; + &object.server + }}, + Leave { |output| { + let Some(object) = &state.get_object_from_client_object::(&output) else { + return; + }; + &object.server + }}, PreferredBufferScale { factor } ] } @@ -363,8 +373,11 @@ impl HandleEvent for Pointer { } => { let do_enter = || { debug!("entering surface ({serial})"); - let surface = state.get_server_surface_from_client(surface.clone()); - self.server.enter(serial, surface, surface_x, surface_y); + if let Some(surface) = state.get_server_surface_from_client(surface.clone()) { + self.server.enter(serial, surface, surface_x, surface_y) + } else { + warn!("could not enter surface: stale surface"); + } }; let surface_key: ObjectKey = surface.data().copied().unwrap(); let surface_data: &SurfaceData = state.objects[surface_key].as_ref(); @@ -400,8 +413,11 @@ impl HandleEvent for Pointer { } debug!("leaving surface ({serial})"); self.pending_enter.0.take(); - self.server - .leave(serial, state.get_server_surface_from_client(surface)); + if let Some(surface) = state.get_server_surface_from_client(surface) { + self.server.leave(serial, surface); + } else { + warn!("could not leave surface: stale surface"); + } } client::wl_pointer::Event::Motion { time, @@ -434,13 +450,23 @@ impl HandleEvent for Pointer { self.server, event: client::wl_pointer::Event => [ Enter { serial, - |surface| state.get_server_surface_from_client(surface), + |surface| { + let Some(surface_data) = state.get_server_surface_from_client(surface) else { + return; + }; + surface_data + }, surface_x, surface_y }, Leave { serial, - |surface| state.get_server_surface_from_client(surface) + |surface| { + let Some(surface_data) = state.get_server_surface_from_client(surface) else { + return; + }; + surface_data + } }, Motion { time, @@ -500,8 +526,9 @@ impl HandleEvent for Keyboard { keys, } => { state.last_kb_serial = Some(serial); - self.server - .enter(serial, state.get_server_surface_from_client(surface), keys); + if let Some(surface_data) = state.get_server_surface_from_client(surface) { + self.server.enter(serial, surface_data, keys); + } } _ => simple_event_shunt! { self.server, event: client::wl_keyboard::Event => [ @@ -516,7 +543,10 @@ impl HandleEvent for Keyboard { if !surface.is_alive() { return; } - state.get_server_surface_from_client(surface) + let Some(surface_data) = state.get_server_surface_from_client(surface) else { + return; + }; + surface_data } }, Key { @@ -551,7 +581,12 @@ impl HandleEvent for Touch { Down { serial, time, - |surface| state.get_server_surface_from_client(surface), + |surface| { + let Some(surface_data) = state.get_server_surface_from_client(surface) else { + return; + }; + surface_data + }, id, x, y diff --git a/src/server/mod.rs b/src/server/mod.rs index c848048..f03f1c4 100644 --- a/src/server/mod.rs +++ b/src/server/mod.rs @@ -496,13 +496,13 @@ impl ServerState { handle_globals::(&self.dh, globals.iter()); } - fn get_object_from_client_object(&self, proxy: &P) -> &T + fn get_object_from_client_object(&self, proxy: &P) -> Option<&T> where for<'a> &'a T: TryFrom<&'a Object, Error = String>, Globals: wayland_client::Dispatch, { let key: ObjectKey = proxy.data().copied().unwrap(); - self.objects[key].as_ref() + Some(self.objects.get(key)?.as_ref()) } pub fn new_window( @@ -539,9 +539,13 @@ impl ServerState { return; }; if let Some(key) = win.surface_key { - let surface: &SurfaceData = self.objects[key].as_ref(); - if let Some(SurfaceRole::Toplevel(Some(data))) = &surface.role { - data.toplevel.set_title(title.name().to_string()); + if let Some(object) = self.objects.get(key) { + let surface: &SurfaceData = object.as_ref(); + if let Some(SurfaceRole::Toplevel(Some(data))) = &surface.role { + data.toplevel.set_title(title.name().to_string()); + } + } else { + warn!("could not set window title: stale surface"); } } } @@ -551,9 +555,13 @@ impl ServerState { let class = win.attrs.class.insert(class); if let Some(key) = win.surface_key { - let surface: &SurfaceData = self.objects[key].as_ref(); - if let Some(SurfaceRole::Toplevel(Some(data))) = &surface.role { - data.toplevel.set_app_id(class.to_string()); + if let Some(object) = self.objects.get(key) { + let surface: &SurfaceData = object.as_ref(); + if let Some(SurfaceRole::Toplevel(Some(data))) = &surface.role { + data.toplevel.set_app_id(class.to_string()); + } + } else { + warn!("could not set window class: stale surface"); } } } @@ -568,15 +576,19 @@ impl ServerState { if win.attrs.size_hints.is_none() || *win.attrs.size_hints.as_ref().unwrap() != hints { debug!("setting {window:?} hints {hints:?}"); - if let Some(surface) = win.surface_key { - let surface: &SurfaceData = self.objects[surface].as_ref(); - if let Some(SurfaceRole::Toplevel(Some(data))) = &surface.role { - if let Some(min_size) = &hints.min_size { - data.toplevel.set_min_size(min_size.width, min_size.height); - } - if let Some(max_size) = &hints.max_size { - data.toplevel.set_max_size(max_size.width, max_size.height); + if let Some(key) = win.surface_key { + if let Some(object) = self.objects.get(key) { + let surface: &SurfaceData = object.as_ref(); + if let Some(SurfaceRole::Toplevel(Some(data))) = &surface.role { + if let Some(min_size) = &hints.min_size { + data.toplevel.set_min_size(min_size.width, min_size.height); + } + if let Some(max_size) = &hints.max_size { + data.toplevel.set_max_size(max_size.width, max_size.height); + } } + } else { + warn!("could not set size hint on {window:?}: stale surface") } } win.attrs.size_hints = Some(hints); @@ -620,7 +632,11 @@ impl ServerState { win.mapped = false; if let Some(key) = win.surface_key.take() { - let surface: &mut SurfaceData = self.objects[key].as_mut(); + let Some(object) = self.objects.get_mut(key) else { + warn!("could not unmap {window:?}: stale surface"); + return; + }; + let surface: &mut SurfaceData = object.as_mut(); surface.destroy_role(); } } @@ -631,7 +647,11 @@ impl ServerState { warn!("Tried to set window without surface fullscreen: {window:?}"); return; }; - let surface: &mut SurfaceData = self.objects[key].as_mut(); + let Some(object) = self.objects.get_mut(key) else { + warn!("Could not set fullscreen on {window:?}: stale surface"); + return; + }; + let surface: &mut SurfaceData = object.as_mut(); let Some(SurfaceRole::Toplevel(Some(ref toplevel))) = surface.role else { warn!("Tried to set an unmapped toplevel or non toplevel fullscreen: {window:?}"); return; @@ -692,10 +712,13 @@ impl ServerState { let client_events = std::mem::take(&mut self.clientside.globals.events); for (key, event) in client_events { - let object = &mut self.objects[key]; + let Some(object) = &mut self.objects.get_mut(key) else { + warn!("could not handle clientside event: stale surface"); + continue; + }; let mut object = object.0.take().unwrap(); object.handle_event(event, self); - let ret = self.objects[key].0.replace(object); + let ret = self.objects[key].0.replace(object); // safe indexed access? debug_assert!(ret.is_none()); } @@ -884,16 +907,22 @@ impl ServerState { } } - fn get_server_surface_from_client(&self, surface: client::wl_surface::WlSurface) -> &WlSurface { + fn get_server_surface_from_client( + &self, + surface: client::wl_surface::WlSurface, + ) -> Option<&WlSurface> { let key: &ObjectKey = surface.data().unwrap(); - let surface: &SurfaceData = self.objects[*key].as_ref(); - &surface.server + let surface: &SurfaceData = self.objects.get(*key)?.as_ref(); + Some(&surface.server) } - fn get_client_surface_from_server(&self, surface: WlSurface) -> &client::wl_surface::WlSurface { + fn get_client_surface_from_server( + &self, + surface: WlSurface, + ) -> Option<&client::wl_surface::WlSurface> { let key: &ObjectKey = surface.data().unwrap(); - let surface: &SurfaceData = self.objects[*key].as_ref(); - &surface.client + let surface: &SurfaceData = self.objects.get(*key)?.as_ref(); + Some(&surface.client) } fn close_x_window(&mut self, window: x::Window) {