From bd60b02d8c1f06a3e2b17b85d20509956f76ded6 Mon Sep 17 00:00:00 2001 From: John Nunley Date: Fri, 1 Mar 2024 01:11:28 -0800 Subject: [PATCH 1/3] On X11, filter out tiny device mouse events Usually, if mouse events are equal to (0, 0) we filter them out. However, if the event is very close to zero it will still be given to the user. In some cases this can be caused by bad float math on the X11 server side. Fix it by filtering absolute values smaller than floating point epsilon. Signed-off-by: John Nunley Closes: #3500 --- CHANGELOG.md | 2 + .../linux/x11/event_processor.rs | 16 +++--- src/platform_impl/linux/x11/util/mod.rs | 3 +- src/platform_impl/linux/x11/util/mouse.rs | 52 +++++++++++++++++++ 4 files changed, 64 insertions(+), 9 deletions(-) create mode 100644 src/platform_impl/linux/x11/util/mouse.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 622ad2bbd9..0531c281e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ Unreleased` header. # Unreleased +- On X11, filter close to zero values in mouse device events + # 0.29.11 - On Wayland, fix DeviceEvent::Motion not being sent diff --git a/src/platform_impl/linux/x11/event_processor.rs b/src/platform_impl/linux/x11/event_processor.rs index b809768443..6882f21489 100644 --- a/src/platform_impl/linux/x11/event_processor.rs +++ b/src/platform_impl/linux/x11/event_processor.rs @@ -1502,8 +1502,8 @@ impl EventProcessor { let mask = unsafe { slice::from_raw_parts(xev.valuators.mask, xev.valuators.mask_len as usize) }; let mut value = xev.raw_values; - let mut mouse_delta = (0.0, 0.0); - let mut scroll_delta = (0.0, 0.0); + let mut mouse_delta = util::Delta::default(); + let mut scroll_delta = util::Delta::default(); for i in 0..xev.valuators.mask_len * 8 { if !xinput2::XIMaskIsSet(mask, i) { continue; @@ -1513,10 +1513,10 @@ impl EventProcessor { // We assume that every XInput2 device with analog axes is a pointing device emitting // relative coordinates. match i { - 0 => mouse_delta.0 = x, - 1 => mouse_delta.1 = x, - 2 => scroll_delta.0 = x as f32, - 3 => scroll_delta.1 = x as f32, + 0 => mouse_delta.set_x(x), + 1 => mouse_delta.set_y(x), + 2 => scroll_delta.set_x(x as f32), + 3 => scroll_delta.set_y(x as f32), _ => {} } @@ -1532,7 +1532,7 @@ impl EventProcessor { value = unsafe { value.offset(1) }; } - if mouse_delta != (0.0, 0.0) { + if let Some(mouse_delta) = mouse_delta.consume() { let event = Event::DeviceEvent { device_id: did, event: DeviceEvent::MouseMotion { delta: mouse_delta }, @@ -1540,7 +1540,7 @@ impl EventProcessor { callback(&self.target, event); } - if scroll_delta != (0.0, 0.0) { + if let Some(scroll_delta) = scroll_delta.consume() { let event = Event::DeviceEvent { device_id: did, event: DeviceEvent::MouseWheel { diff --git a/src/platform_impl/linux/x11/util/mod.rs b/src/platform_impl/linux/x11/util/mod.rs index 937671aa58..f33dc763ca 100644 --- a/src/platform_impl/linux/x11/util/mod.rs +++ b/src/platform_impl/linux/x11/util/mod.rs @@ -15,13 +15,14 @@ mod icon; mod input; pub mod keys; pub(crate) mod memory; +mod mouse; mod randr; mod window_property; mod wm; mod xmodmap; pub use self::{ - geometry::*, hint::*, input::*, window_property::*, wm::*, xmodmap::ModifierKeymap, + geometry::*, hint::*, input::*, mouse::*, window_property::*, wm::*, xmodmap::ModifierKeymap, }; use super::{atoms::*, ffi, VoidCookie, X11Error, XConnection, XError}; diff --git a/src/platform_impl/linux/x11/util/mouse.rs b/src/platform_impl/linux/x11/util/mouse.rs new file mode 100644 index 0000000000..212ea57267 --- /dev/null +++ b/src/platform_impl/linux/x11/util/mouse.rs @@ -0,0 +1,52 @@ +//! Utilities for handling mouse events. + +/// Recorded mouse delta designed to filter out noise. +pub struct Delta { + x: T, + y: T, +} + +impl Default for Delta { + fn default() -> Self { + Self { + x: Default::default(), + y: Default::default(), + } + } +} + +impl Delta { + pub(crate) fn set_x(&mut self, x: T) { + self.x = x; + } + + pub(crate) fn set_y(&mut self, y: T) { + self.y = y; + } +} + +macro_rules! consume { + ($this:expr, $ty:ty) => {{ + let this = $this; + let (x, y) = match (this.x.abs() < <$ty>::EPSILON, this.y.abs() < <$ty>::EPSILON) { + (true, true) => return None, + (true, false) => (this.x, 0.0), + (false, true) => (0.0, this.y), + (false, false) => (this.x, this.y), + }; + + Some((x, y)) + }}; +} + +impl Delta { + pub(crate) fn consume(self) -> Option<(f32, f32)> { + consume!(self, f32) + } +} + +impl Delta { + pub(crate) fn consume(self) -> Option<(f64, f64)> { + consume!(self, f64) + } +} From c4b5c5c2f4236e9c2f8ecf5c26afe2b303faced0 Mon Sep 17 00:00:00 2001 From: Kirill Chibisov Date: Fri, 1 Mar 2024 13:40:20 +0400 Subject: [PATCH 2/3] 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; From a19f7ca70ff0a40e067fcf9630390884d24ba5f6 Mon Sep 17 00:00:00 2001 From: Kirill Chibisov Date: Fri, 1 Mar 2024 13:45:01 +0400 Subject: [PATCH 3/3] Winit version 0.29.12 --- CHANGELOG.md | 2 ++ Cargo.toml | 2 +- README.md | 4 ++-- 3 files changed, 5 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf96ccc714..81510c43da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ Unreleased` header. # Unreleased +# 0.29.12 + - On X11, fix use after free during xinput2 handling. - On X11, filter close to zero values in mouse device events diff --git a/Cargo.toml b/Cargo.toml index 18f5a9caf0..6e7c461647 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "winit" -version = "0.29.11" +version = "0.29.12" authors = ["The winit contributors", "Pierre Krieger "] description = "Cross-platform window creation library." edition = "2021" diff --git a/README.md b/README.md index 53b07354f6..17e181e2ef 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ ```toml [dependencies] -winit = "0.29.11" +winit = "0.29.12" ``` ## [Documentation](https://docs.rs/winit) @@ -156,7 +156,7 @@ For more details, refer to these `android-activity` [example applications](https If your application is currently based on `NativeActivity` via the `ndk-glue` crate and building with `cargo apk`, then the minimal changes would be: 1. Remove `ndk-glue` from your `Cargo.toml` -2. Enable the `"android-native-activity"` feature for Winit: `winit = { version = "0.29.11", features = [ "android-native-activity" ] }` +2. Enable the `"android-native-activity"` feature for Winit: `winit = { version = "0.29.12", features = [ "android-native-activity" ] }` 3. Add an `android_main` entrypoint (as above), instead of using the '`[ndk_glue::main]` proc macro from `ndk-macros` (optionally add a dependency on `android_logger` and initialize logging as above). 4. Pass a clone of the `AndroidApp` that your application receives to Winit when building your event loop (as shown above).