meh / rust-xcb-util

Rust bindings and wrappers for XCB utility functions.
MIT License
27 stars 17 forks source link

Unsoundness in xcb_util::image::shm::Image #13

Open cmaves opened 4 years ago

cmaves commented 4 years ago

If the xcb::Connection, that is used to create the xcb::image::shm::Image, is dropped before the Image then a double free will occur causing a crash.

A short program demonstrating this:

use xcb;
use xcb_util::image::shm;

fn main() {
    let (conn, _) = xcb::Connection::connect(None).unwrap();
    let shm_img = shm::create(&conn, 24, 100, 100).unwrap();
    drop(conn);
        drop(shm_img);
        println!("Hello, world!");
}

PS: Thanks for making this crate. Its awesome.

meh commented 4 years ago

:thinking:

The issue here is pervasive with the xcb crate as well I feel, there are two options:

  1. Demand an Rc<xcb::Connection> or Arc<xcb::Connection>, but then without HKTs picking which one of the two becomes ugly, so it'd likely have to be an Arc<xcb::Connection> which is overhead.
  2. Add a lifetime requirement to shm::Image which makes the API more complex and makes it hard to store an shm::Image into other types.
  3. Lie on the floor and cry.

Which way of suffering would you prefer in your code for instance? If you showed me how you're using shm::Image I can see what the least painful solution is.

cmaves commented 4 years ago

I was using it in a crate to set the root window's background pixmap. I took your image module and changed it to use lifetimes but I imagine the Rc<xcb::Connection> or Arc<xcb::Connection> would be more ergonomic for other users of the crate. Migrating existing dependent crates to use a Arc<xcb::Connection> solution would likely be trivial but satisfying a new lifetime requirement could be pain, especially you're using threading.