From 36fba6a76bda67e1d9108ef245ceb0e16cd0e45e Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Sat, 28 Sep 2024 22:17:13 -0700 Subject: [PATCH 1/2] Fix unsoundness in CGEventTap API --- core-graphics/src/event.rs | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/core-graphics/src/event.rs b/core-graphics/src/event.rs index 96c3e967..ecddf7c0 100644 --- a/core-graphics/src/event.rs +++ b/core-graphics/src/event.rs @@ -6,7 +6,7 @@ use bitflags::bitflags; use core::ffi::{c_ulong, c_void}; use core_foundation::{ base::{CFRelease, CFRetain, CFTypeID, TCFType}, - mach_port::{CFMachPort, CFMachPortRef}, + mach_port::{CFMachPort, CFMachPortInvalidate, CFMachPortRef}, }; use foreign_types::{foreign_type, ForeignType}; use std::mem::ManuallyDrop; @@ -417,7 +417,7 @@ macro_rules! CGEventMaskBit { } pub type CGEventTapProxy = *const c_void; -pub type CGEventTapCallBackFn<'tap_life> = +type CGEventTapCallBackFn<'tap_life> = Box Option + 'tap_life>; type CGEventTapCallBackInternal = unsafe extern "C" fn( proxy: CGEventTapProxy, @@ -458,9 +458,9 @@ unsafe extern "C" fn cg_event_tap_callback_internal( /// ) { /// Ok(tap) => unsafe { /// let loop_source = tap -/// .mach_port +/// .mach_port() /// .create_runloop_source(0) -/// .expect("Somethings is bad "); +/// .expect("Runloop source creation failed"); /// current.add_source(&loop_source, kCFRunLoopCommonModes); /// tap.enable(); /// CFRunLoop::run_current(); @@ -469,9 +469,8 @@ unsafe extern "C" fn cg_event_tap_callback_internal( /// } /// ``` pub struct CGEventTap<'tap_life> { - pub mach_port: CFMachPort, - pub callback_ref: - Box Option + 'tap_life>, + mach_port: CFMachPort, + _callback: Box>, } impl<'tap_life> CGEventTap<'tap_life> { @@ -487,7 +486,7 @@ impl<'tap_life> CGEventTap<'tap_life> { .fold(CGEventType::Null as CGEventMask, |mask, &etype| { mask | CGEventMaskBit!(etype) }); - let cb = Box::new(Box::new(callback) as CGEventTapCallBackFn); + let cb: Box = Box::new(Box::new(callback)); let cbr = Box::into_raw(cb); unsafe { let event_tap_ref = CGEventTapCreate( @@ -502,7 +501,7 @@ impl<'tap_life> CGEventTap<'tap_life> { if !event_tap_ref.is_null() { Ok(Self { mach_port: (CFMachPort::wrap_under_create_rule(event_tap_ref)), - callback_ref: Box::from_raw(cbr), + _callback: Box::from_raw(cbr), }) } else { let _ = Box::from_raw(cbr); @@ -511,11 +510,21 @@ impl<'tap_life> CGEventTap<'tap_life> { } } + pub fn mach_port(&self) -> &CFMachPort { + &self.mach_port + } + pub fn enable(&self) { unsafe { CGEventTapEnable(self.mach_port.as_concrete_TypeRef(), true) } } } +impl Drop for CGEventTap<'_> { + fn drop(&mut self) { + unsafe { CFMachPortInvalidate(self.mach_port.as_CFTypeRef() as *mut _) }; + } +} + foreign_type! { #[doc(hidden)] pub unsafe type CGEvent { From fb3708c66f6403da9d5a112f08366583ada7f0cc Mon Sep 17 00:00:00 2001 From: Tyler Mandry Date: Sat, 28 Sep 2024 23:08:24 -0700 Subject: [PATCH 2/2] Fix lifetime unsoundness in CGEventTap, add ::with --- core-graphics/src/event.rs | 59 +++++++++++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 14 deletions(-) diff --git a/core-graphics/src/event.rs b/core-graphics/src/event.rs index ecddf7c0..77b699e7 100644 --- a/core-graphics/src/event.rs +++ b/core-graphics/src/event.rs @@ -443,44 +443,75 @@ unsafe extern "C" fn cg_event_tap_callback_internal( } /// ```no_run -///use core_foundation::runloop::{kCFRunLoopCommonModes, CFRunLoop}; -///use core_graphics::event::{CGEventTap, CGEventTapLocation, CGEventTapPlacement, CGEventTapOptions, CGEventType}; -///let current = CFRunLoop::get_current(); -///match CGEventTap::new( +/// use core_foundation::runloop::{kCFRunLoopCommonModes, CFRunLoop}; +/// use core_graphics::event::{CGEventTap, CGEventTapLocation, CGEventTapPlacement, CGEventTapOptions, CGEventType}; +/// let current = CFRunLoop::get_current(); +/// +/// CGEventTap::with( /// CGEventTapLocation::HID, /// CGEventTapPlacement::HeadInsertEventTap, /// CGEventTapOptions::Default, /// vec![CGEventType::MouseMoved], -/// |_a, _b, d| { -/// println!("{:?}", d.location()); +/// |_proxy, _type, event| { +/// println!("{:?}", event.location()); /// None /// }, -/// ) { -/// Ok(tap) => unsafe { +/// |tap| { /// let loop_source = tap /// .mach_port() /// .create_runloop_source(0) /// .expect("Runloop source creation failed"); -/// current.add_source(&loop_source, kCFRunLoopCommonModes); +/// current.add_source(&loop_source, unsafe { kCFRunLoopCommonModes }); /// tap.enable(); /// CFRunLoop::run_current(); /// }, -/// Err(_) => (assert!(false)), -/// } +/// ).expect("Failed to install event tap"); /// ``` pub struct CGEventTap<'tap_life> { mach_port: CFMachPort, _callback: Box>, } -impl<'tap_life> CGEventTap<'tap_life> { - pub fn new Option + 'tap_life>( +impl CGEventTap<'static> { + pub fn new Option + 'static>( tap: CGEventTapLocation, place: CGEventTapPlacement, options: CGEventTapOptions, events_of_interest: std::vec::Vec, callback: F, - ) -> Result, ()> { + ) -> Result { + // SAFETY: callback is 'static so even if this object is forgotten it + // will be valid to call. + unsafe { Self::new_unchecked(tap, place, options, events_of_interest, callback) } + } +} + +impl<'tap_life> CGEventTap<'tap_life> { + pub fn with( + tap: CGEventTapLocation, + place: CGEventTapPlacement, + options: CGEventTapOptions, + events_of_interest: std::vec::Vec, + callback: impl Fn(CGEventTapProxy, CGEventType, &CGEvent) -> Option + 'tap_life, + with_fn: impl FnOnce(&Self) -> R, + ) -> Result { + // SAFETY: We are okay to bypass the 'static restriction because the + // event tap is dropped before returning. The callback therefore cannot + // be called after its lifetime expires. + let event_tap: Self = + unsafe { Self::new_unchecked(tap, place, options, events_of_interest, callback)? }; + Ok(with_fn(&event_tap)) + } + + /// Caller is responsible for ensuring that this object is dropped before + /// `'tap_life` expires. + pub unsafe fn new_unchecked( + tap: CGEventTapLocation, + place: CGEventTapPlacement, + options: CGEventTapOptions, + events_of_interest: std::vec::Vec, + callback: impl Fn(CGEventTapProxy, CGEventType, &CGEvent) -> Option + 'tap_life, + ) -> Result { let event_mask: CGEventMask = events_of_interest .iter() .fold(CGEventType::Null as CGEventMask, |mask, &etype| {