On X11, fix use after free during xinput2 processing

Fixes #3536.
rust-windowing · Mar 1, 2024 · 58c89c1 · 58c89c1
1 parent 2dae807
commit 58c89c1
Show file tree

Hide file tree

Showing 5 changed files with 84 additions and 53 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,7 @@ Unreleased` header.
 
 # Unreleased
 
+- On X11, fix use after free during xinput2 handling.
 - On X11, filter close to zero values in mouse device events
 
 # 0.29.11

diff --git a/src/platform_impl/linux/x11/event_processor.rs b/src/platform_impl/linux/x11/event_processor.rs
@@ -33,9 +33,10 @@ use crate::platform_impl::platform::common::xkb::Context;
 use crate::platform_impl::platform::x11::ime::{ImeEvent, ImeEventReceiver, ImeRequest};
 use crate::platform_impl::platform::x11::EventLoopWindowTarget;
 use crate::platform_impl::platform::EventLoopWindowTarget as PlatformEventLoopWindowTarget;
+use crate::platform_impl::x11::util::cookie::GenericEventCookie;
 use crate::platform_impl::x11::{
     atoms::*, mkdid, mkwid, util, CookieResultExt, Device, DeviceId, DeviceInfo, Dnd, DndState,
-    GenericEventCookie, ImeReceiver, ScrollOrientation, UnownedWindow, WindowId,
+    ImeReceiver, ScrollOrientation, UnownedWindow, WindowId,
 };
 
 /// The maximum amount of X modifiers to replay.
@@ -184,22 +185,23 @@ impl<T: 'static> EventProcessor<T> {
             }
             xlib::GenericEvent => {
                 let wt = Self::window_target(&self.target);
-                let xev = match GenericEventCookie::from_event(&wt.xconn, *xev) {
-                    Some(xev) if xev.cookie.extension as u8 == self.xi2ext.major_opcode => {
-                        xev.cookie
-                    }
-                    _ => return,
-                };
+                let xev: GenericEventCookie =
+                    match GenericEventCookie::from_event(wt.xconn.clone(), *xev) {
+                        Some(xev) if xev.extension() == self.xi2ext.major_opcode => xev,
+                        _ => return,
+                    };
+
+                let evtype = xev.evtype();
 
-                match xev.evtype {
+                match evtype {
                     ty @ xinput2::XI_ButtonPress | ty @ xinput2::XI_ButtonRelease => {
                         let state = if ty == xinput2::XI_ButtonPress {
                             ElementState::Pressed
                         } else {
                             ElementState::Released
                         };
 
-                        let xev: &XIDeviceEvent = unsafe { &*(xev.data as *const _) };
+                        let xev: &XIDeviceEvent = unsafe { xev.as_event() };
                         self.update_mods_from_xinput2_event(
                             &xev.mods,
                             &xev.group,
@@ -209,7 +211,7 @@ impl<T: 'static> EventProcessor<T> {
                         self.xinput2_button_input(xev, state, &mut callback);
                     }
                     xinput2::XI_Motion => {
-                        let xev: &XIDeviceEvent = unsafe { &*(xev.data as *const _) };
+                        let xev: &XIDeviceEvent = unsafe { xev.as_event() };
                         self.update_mods_from_xinput2_event(
                             &xev.mods,
                             &xev.group,
@@ -219,11 +221,11 @@ impl<T: 'static> EventProcessor<T> {
                         self.xinput2_mouse_motion(xev, &mut callback);
                     }
                     xinput2::XI_Enter => {
-                        let xev: &XIEnterEvent = unsafe { &*(xev.data as *const _) };
+                        let xev: &XIEnterEvent = unsafe { xev.as_event() };
                         self.xinput2_mouse_enter(xev, &mut callback);
                     }
                     xinput2::XI_Leave => {
-                        let xev: &XILeaveEvent = unsafe { &*(xev.data as *const _) };
+                        let xev: &XILeaveEvent = unsafe { xev.as_event() };
                         self.update_mods_from_xinput2_event(
                             &xev.mods,
                             &xev.group,
@@ -233,51 +235,51 @@ impl<T: 'static> EventProcessor<T> {
                         self.xinput2_mouse_left(xev, &mut callback);
                     }
                     xinput2::XI_FocusIn => {
-                        let xev: &XIFocusInEvent = unsafe { &*(xev.data as *const _) };
+                        let xev: &XIFocusInEvent = unsafe { xev.as_event() };
                         self.xinput2_focused(xev, &mut callback);
                     }
                     xinput2::XI_FocusOut => {
-                        let xev: &XIFocusOutEvent = unsafe { &*(xev.data as *const _) };
+                        let xev: &XIFocusOutEvent = unsafe { xev.as_event() };
                         self.xinput2_unfocused(xev, &mut callback);
                     }
                     xinput2::XI_TouchBegin | xinput2::XI_TouchUpdate | xinput2::XI_TouchEnd => {
-                        let phase = match xev.evtype {
+                        let phase = match evtype {
                             xinput2::XI_TouchBegin => TouchPhase::Started,
                             xinput2::XI_TouchUpdate => TouchPhase::Moved,
                             xinput2::XI_TouchEnd => TouchPhase::Ended,
                             _ => unreachable!(),
                         };
 
-                        let xev: &XIDeviceEvent = unsafe { &*(xev.data as *const _) };
+                        let xev: &XIDeviceEvent = unsafe { xev.as_event() };
                         self.xinput2_touch(xev, phase, &mut callback);
                     }
                     xinput2::XI_RawButtonPress | xinput2::XI_RawButtonRelease => {
-                        let state = match xev.evtype {
+                        let state = match evtype {
                             xinput2::XI_RawButtonPress => ElementState::Pressed,
                             xinput2::XI_RawButtonRelease => ElementState::Released,
                             _ => unreachable!(),
                         };
 
-                        let xev: &XIRawEvent = unsafe { &*(xev.data as *const _) };
+                        let xev: &XIRawEvent = unsafe { xev.as_event() };
                         self.xinput2_raw_button_input(xev, state, &mut callback);
                     }
                     xinput2::XI_RawMotion => {
-                        let xev: &XIRawEvent = unsafe { &*(xev.data as *const _) };
+                        let xev: &XIRawEvent = unsafe { xev.as_event() };
                         self.xinput2_raw_mouse_motion(xev, &mut callback);
                     }
                     xinput2::XI_RawKeyPress | xinput2::XI_RawKeyRelease => {
-                        let state = match xev.evtype {
+                        let state = match evtype {
                             xinput2::XI_RawKeyPress => ElementState::Pressed,
                             xinput2::XI_RawKeyRelease => ElementState::Released,
                             _ => unreachable!(),
                         };
 
-                        let xev: &xinput2::XIRawEvent = unsafe { &*(xev.data as *const _) };
+                        let xev: &xinput2::XIRawEvent = unsafe { xev.as_event() };
                         self.xinput2_raw_key_input(xev, state, &mut callback);
                     }
 
                     xinput2::XI_HierarchyChanged => {
-                        let xev: &XIHierarchyEvent = unsafe { &*(xev.data as *const _) };
+                        let xev: &XIHierarchyEvent = unsafe { xev.as_event() };
                         self.xinput2_hierarchy_changed(xev, &mut callback);
                     }
                     _ => {}

diff --git a/src/platform_impl/linux/x11/mod.rs b/src/platform_impl/linux/x11/mod.rs
@@ -69,6 +69,9 @@ const ALL_DEVICES: u16 = 0;
 const ALL_MASTER_DEVICES: u16 = 1;
 const ICONIC_STATE: u32 = 3;
 
+/// The underlying x11rb connection that we are using.
+type X11rbConnection = x11rb::xcb_ffi::XCBConnection;
+
 type X11Source = Generic<BorrowedFd<'static>>;
 
 struct WakeSender<T> {
@@ -989,9 +992,6 @@ impl From<xsettings::ParserError> for X11Error {
     }
 }
 
-/// The underlying x11rb connection that we are using.
-type X11rbConnection = x11rb::xcb_ffi::XCBConnection;
-
 /// Type alias for a void cookie.
 type VoidCookie<'a> = x11rb::cookie::VoidCookie<'a, X11rbConnection>;
 
@@ -1007,34 +1007,6 @@ impl<'a, E: fmt::Debug> CookieResultExt for Result<VoidCookie<'a>, E> {
     }
 }
 
-/// XEvents of type GenericEvent store their actual data in an XGenericEventCookie data structure. This is a wrapper to
-/// extract the cookie from a GenericEvent XEvent and release the cookie data once it has been processed
-struct GenericEventCookie<'a> {
-    xconn: &'a XConnection,
-    cookie: ffi::XGenericEventCookie,
-}
-
-impl<'a> GenericEventCookie<'a> {
-    fn from_event(xconn: &XConnection, event: ffi::XEvent) -> Option<GenericEventCookie<'_>> {
-        unsafe {
-            let mut cookie: ffi::XGenericEventCookie = From::from(event);
-            if (xconn.xlib.XGetEventData)(xconn.display, &mut cookie) == ffi::True {
-                Some(GenericEventCookie { xconn, cookie })
-            } else {
-                None
-            }
-        }
-    }
-}
-
-impl<'a> Drop for GenericEventCookie<'a> {
-    fn drop(&mut self) {
-        unsafe {
-            (self.xconn.xlib.XFreeEventData)(self.xconn.display, &mut self.cookie);
-        }
-    }
-}
-
 fn mkwid(w: xproto::Window) -> crate::window::WindowId {
     crate::window::WindowId(crate::platform_impl::platform::WindowId(w as _))
 }

diff --git a/src/platform_impl/linux/x11/util/cookie.rs b/src/platform_impl/linux/x11/util/cookie.rs
@@ -0,0 +1,55 @@
+use std::ffi::c_int;
+use std::sync::Arc;
+
+use x11_dl::xlib::{self, XEvent, XGenericEventCookie};
+
+use crate::platform_impl::x11::XConnection;
+
+/// XEvents of type GenericEvent store their actual data in an XGenericEventCookie data structure.
+/// This is a wrapper to extract the cookie from a GenericEvent XEvent and release the cookie data
+/// once it has been processed
+pub struct GenericEventCookie {
+    cookie: XGenericEventCookie,
+    xconn: Arc<XConnection>,
+}
+
+impl GenericEventCookie {
+    pub fn from_event(xconn: Arc<XConnection>, event: XEvent) -> Option<GenericEventCookie> {
+        unsafe {
+            let mut cookie: XGenericEventCookie = From::from(event);
+            if (xconn.xlib.XGetEventData)(xconn.display, &mut cookie) == xlib::True {
+                Some(GenericEventCookie { cookie, xconn })
+            } else {
+                None
+            }
+        }
+    }
+
+    #[inline]
+    pub fn extension(&self) -> u8 {
+        self.cookie.extension as u8
+    }
+
+    #[inline]
+    pub fn evtype(&self) -> c_int {
+        self.cookie.evtype
+    }
+
+    /// Borrow inner event data as `&T`.
+    ///
+    /// ## SAFETY
+    ///
+    /// The caller must ensure that the event has the `T` inside of it.
+    #[inline]
+    pub unsafe fn as_event<T>(&self) -> &T {
+        unsafe { &*(self.cookie.data as *const _) }
+    }
+}
+
+impl Drop for GenericEventCookie {
+    fn drop(&mut self) {
+        unsafe {
+            (self.xconn.xlib.XFreeEventData)(self.xconn.display, &mut self.cookie);
+        }
+    }
+}
diff --git a/src/platform_impl/linux/x11/util/mod.rs b/src/platform_impl/linux/x11/util/mod.rs
@@ -8,6 +8,7 @@ use std::{
 };
 
 mod client_msg;
+pub mod cookie;
 mod cursor;
 mod geometry;
 mod hint;