Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

On X11, fix use after free during xinput2 processing #3540

Merged
merged 1 commit into from
Mar 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
- Move `dpi` types to its own crate, and re-export it from the root crate.
- Implement `Sync` for `EventLoopProxy<T: Send>`.
Expand Down
46 changes: 24 additions & 22 deletions src/platform_impl/linux/x11/event_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::ActiveEventLoop;
use crate::platform_impl::platform::ActiveEventLoop as PlatformActiveEventLoop;
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.
Expand Down Expand Up @@ -184,22 +185,23 @@ 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
} 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,
Expand All @@ -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,
Expand All @@ -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,
Expand All @@ -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);
}
_ => {}
Expand Down
34 changes: 3 additions & 31 deletions src/platform_impl/linux/x11/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,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> {
Expand Down Expand Up @@ -985,9 +988,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>;

Expand All @@ -1003,34 +1003,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 _))
}
Expand Down
55 changes: 55 additions & 0 deletions src/platform_impl/linux/x11/util/cookie.rs
Original file line number Diff line number Diff line change
@@ -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);
}
}
}
1 change: 1 addition & 0 deletions src/platform_impl/linux/x11/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use std::{
};

mod client_msg;
pub mod cookie;
mod cursor;
mod geometry;
mod hint;
Expand Down
Loading