leyoda/smithay-clipboard
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix segfault in multithreaded context

Browse source 
Dropping libwayland-client's queue in multithreaded context is safe if
and only if there're no alive proxies on it, otherwise it could result
in use-after-free. For more see [1] and [2].

[1] - https://gitlab.freedesktop.org/wayland/wayland/-/issues/13
[2] - https://github.com/Smithay/wayland-rs/issues/359
This commit is contained in:
Kirill Chibisov 2020-12-12 18:28:29 +03:00 • committed by GitHub
parent 276aa84cf5
commit f6f5b98235
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 88 additions and 42 deletions
Download patch file Download diff file Expand all files Collapse all files

2
CHANGELOG.md
View file

@ -2,6 +2,8 @@
## Unreleased
- Segfault when dropping clipboard in multithreaded context while main queue is still running
## 0.6.1 -- 2020-10-13
- Crash when failing to write to a clipboard

2
examples/clipboard.rs
View file

@ -223,7 +223,7 @@ fn process_keyboard_event(event: KeyboardEvent, dispatch_data: &mut DispatchData
let contents = dispatch_data
.clipboard
.load()
.unwrap_or_else(|_| String::from("Failed to load primary selection"));
.unwrap_or_else(|_| String::from("Failed to load selection"));
println!("Paste: {}", contents);
}
// Copy primary.

67
src/worker/dispatch_data.rs
View file

@ -1,47 +1,68 @@
use std::collections::VecDeque;
use std::slice::IterMut;
use sctk::reexports::client::protocol::wl_seat::WlSeat;
use super::seat_data::SeatData;
/// Data to track latest seat and serial for clipboard requests.
#[derive(Default)]
pub struct ClipboardDispatchData {
observed_seats: Vec<(WlSeat, u32)>,
last_pos: usize,
/// Seats that our application encountered. The first seat is the latest one we've encountered.
observed_seats: VecDeque<(WlSeat, u32)>,
/// All the seats that were advertised.
seats: Vec<SeatData>,
}
impl ClipboardDispatchData {
/// Builds new `ClipboardDispatchData` with all fields equal to `None`.
pub fn new() -> Self {
Self::default()
pub fn new(seats: Vec<SeatData>) -> Self {
Self { observed_seats: Default::default(), seats }
}
/// Returns the requested seat's data or adds a new one.
pub fn get_seat_data_or_add(&mut self, seat: WlSeat) -> &mut SeatData {
let pos = self.seats.iter().position(|st| st.seat == seat);
let index = pos.unwrap_or_else(|| {
self.seats.push(SeatData::new(seat, None, None));
self.seats.len() - 1
});
&mut self.seats[index]
}
pub fn seats(&mut self) -> IterMut<'_, SeatData> {
self.seats.iter_mut()
}
/// Set the last observed seat.
pub fn set_last_seat(&mut self, seat: WlSeat, serial: u32) {
pub fn set_last_observed_seat(&mut self, seat: WlSeat, serial: u32) {
let pos = self.observed_seats.iter().position(|st| st.0 == seat);
match pos {
let (seat, serial) = match pos {
Some(pos) => {
// Update serial and set the last data we've seen.
self.observed_seats[pos].1 = serial;
self.last_pos = pos;
}
None => {
// Add new seat and mark it as last.
self.last_pos = self.observed_seats.len();
self.observed_seats.push((seat, serial));
}
// We just found that `pos` we're going to remove, so unwrapping is safe.
self.observed_seats.remove(pos).unwrap()
}
None => (seat, serial),
};
// Add seat to front, thus it'll be the latest observed one.
self.observed_seats.push_front((seat, serial));
}
/// Remove the given seat from the observer seats.
pub fn remove_seat(&mut self, seat: WlSeat) {
let pos = self.observed_seats.iter().position(|st| st.0 == seat);
/// Remove the given seat from the observed seats.
pub fn remove_observed_seat(&mut self, seat: WlSeat) {
let pos = match self.observed_seats.iter().position(|st| st.0 == seat) {
Some(pos) => pos,
None => return,
};
if let Some(pos) = pos {
// Remove the seat data.
self.observed_seats.remove(pos);
}
}
/// Return the last observed seat and the serial.
pub fn last_seat(&self) -> Option<&(WlSeat, u32)> {
self.observed_seats.get(self.last_pos)
pub fn last_observed_seat(&self) -> Option<&(WlSeat, u32)> {
self.observed_seats.front()
}
}

10
src/worker/handlers.rs
View file

@ -104,10 +104,10 @@ pub fn pointer_handler(seat: WlSeat, event: PointerEvent, mut dispatch_data: Dis
};
match event {
PointerEvent::Enter { serial, .. } => {
dispatch_data.set_last_seat(seat, serial);
dispatch_data.set_last_observed_seat(seat, serial);
}
PointerEvent::Button { serial, .. } => {
dispatch_data.set_last_seat(seat, serial);
dispatch_data.set_last_observed_seat(seat, serial);
}
_ => {}
}
@ -121,13 +121,13 @@ pub fn keyboard_handler(seat: WlSeat, event: KeyboardEvent, mut dispatch_data: D
};
match event {
KeyboardEvent::Enter { serial, .. } => {
dispatch_data.set_last_seat(seat, serial);
dispatch_data.set_last_observed_seat(seat, serial);
}
KeyboardEvent::Key { serial, .. } => {
dispatch_data.set_last_seat(seat, serial);
dispatch_data.set_last_observed_seat(seat, serial);
}
KeyboardEvent::Leave { .. } => {
dispatch_data.remove_seat(seat);
dispatch_data.remove_observed_seat(seat);
}
KeyboardEvent::Keymap { fd, .. } => {
// Prevent fd leaking.

45
src/worker/mod.rs
View file

@ -143,15 +143,13 @@ fn worker_impl(display: Display, request_rx: Receiver<Command>, reply_tx: Sender
}
// Listen for seats.
let _listener = env.listen_for_seats(move |seat, seat_data, _| {
let detached_seat = seat.detach();
let pos = seats.iter().position(|st| st.seat == detached_seat);
let index = pos.unwrap_or_else(|| {
seats.push(SeatData::new(detached_seat, None, None));
seats.len() - 1
});
let listener = env.listen_for_seats(move |seat, seat_data, mut dispatch_data| {
let dispatch_data = match dispatch_data.get::<ClipboardDispatchData>() {
Some(dispatch_data) => dispatch_data,
None => return,
};
let seat_resources = &mut seats[index];
let seat_resources = dispatch_data.get_seat_data_or_add(seat.detach());
if seat_data.has_keyboard && !seat_data.defunct {
if seat_resources.keyboard.is_none() {
@ -194,7 +192,7 @@ fn worker_impl(display: Display, request_rx: Receiver<Command>, reply_tx: Sender
// Flush the display.
let _ = queue.display().flush();
let mut dispatch_data = ClipboardDispatchData::new();
let mut dispatch_data = ClipboardDispatchData::new(seats);
// Setup sleep amount tracker.
let mut sa_tracker = SleepAmountTracker::new(MAX_TIME_TO_SLEEP, MAX_WARM_WAKEUPS);
@ -219,7 +217,7 @@ fn worker_impl(display: Display, request_rx: Receiver<Command>, reply_tx: Sender
}
// Get latest observed seat and serial.
let (seat, serial) = match dispatch_data.last_seat() {
let (seat, serial) = match dispatch_data.last_observed_seat() {
Some(data) => data,
None => {
handlers::reply_error(&reply_tx, "no focus on a seat.");
@ -284,7 +282,7 @@ fn worker_impl(display: Display, request_rx: Receiver<Command>, reply_tx: Sender
// If some application is trying to spam us when there're no seats, it's likely that
// someone is trying to paste from us.
if dispatch_data.last_seat().is_none() && pending_events != 0 {
if dispatch_data.last_observed_seat().is_none() && pending_events != 0 {
sa_tracker.reset_sleep();
} else {
// Time for thread to sleep.
@ -296,4 +294,29 @@ fn worker_impl(display: Display, request_rx: Receiver<Command>, reply_tx: Sender
sa_tracker.increase_sleep();
}
}
// While everything inside this block is safe, the logic is generally unsafe, since we must
// drop every proxy on the current `queue`, since dropping it in multithreaded context
// could result in use-after-free in libwayland-client.
//
// For more see https://gitlab.freedesktop.org/wayland/wayland/-/issues/13.
#[allow(unused_unsafe)]
unsafe {
for seat in dispatch_data.seats() {
if let Some(pointer) = seat.pointer.take() {
if pointer.as_ref().version() >= 3 {
pointer.release();
}
}
if let Some(keyboard) = seat.keyboard.take() {
if keyboard.as_ref().version() >= 3 {
keyboard.release();
}
}
}
std::mem::drop(listener);
let _ = queue.sync_roundtrip(&mut dispatch_data, |_, _, _| unimplemented!());
let _ = queue.display().flush();
}
}