From 58c89c1ffceed260242dce901914562813f849cb Mon Sep 17 00:00:00 2001 From: Kirill Chibisov Date: Fri, 1 Mar 2024 13:40:20 +0400 Subject: [PATCH] On X11, fix use after free during xinput2 processing Fixes #3536. --- CHANGELOG.md | 1 + .../linux/x11/event_processor.rs | 46 ++++++++-------- src/platform_impl/linux/x11/mod.rs | 34 +----------- src/platform_impl/linux/x11/util/cookie.rs | 55 +++++++++++++++++++ src/platform_impl/linux/x11/util/mod.rs | 1 + 5 files changed, 84 insertions(+), 53 deletions(-) create mode 100644 src/platform_impl/linux/x11/util/cookie.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 0531c281e1..cf96ccc714 100644 --- 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 index 6882f21489..92a71ecaba 100644 --- 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,14 +185,15 @@ impl EventProcessor { } 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 @@ -199,7 +201,7 @@ impl EventProcessor { 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 EventProcessor { 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 EventProcessor { 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 EventProcessor { 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 index aa0442069e..d882bcf7e5 100644 --- 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>; struct WakeSender { @@ -989,9 +992,6 @@ impl From 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, 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> { - 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 new file mode 100644 index 0000000000..ccdfa8cd59 --- /dev/null +++ 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, +} + +impl GenericEventCookie { + pub fn from_event(xconn: Arc, event: XEvent) -> Option { + 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(&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 index f33dc763ca..8df4966b1d 100644 --- 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;