Skip to content

Commit

Permalink
fix(mmserver): try to avoid colliding with the system x11 socket
Browse files Browse the repository at this point in the history
A real xserver sometimes creates "abstract sockets", which transcend the
filesystem and can leak into our container (because we don't use a
network namespace). That means when running on a system with the system
xserver at :1, we would potentially end up colliding with it, and the app
would connect to the system xserver instead of xwayland.

Note that we can't just use DISPLAY=/path/to/internal/socket, because
that's not supported on even some recent libxcb versions.

However, we can avoid this specific situation by just checking whether
an abstract socket exists for a given display number, and choosing a
higher number if so.
  • Loading branch information
colinmarc committed Feb 7, 2025
1 parent 39fa20c commit 5c554cc
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 17 deletions.
71 changes: 55 additions & 16 deletions mm-server/src/session/compositor/xwayland.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

mod xwm;
use std::{
io::Read as _,
io::{self, Read as _},
os::fd::{AsFd, AsRawFd as _},
path::{Path, PathBuf},
sync::Arc,
Expand All @@ -22,16 +22,54 @@ use crate::{
};

pub struct XWayland {
pub display_socket: PathBuf,
pub display_socket: DisplaySocket,
pub displayfd_recv: mio::unix::pipe::Receiver,
pub child: ContainerHandle,

extern_socket_dir: PathBuf,
xwm_socket: Option<mio::net::UnixStream>,
}

// Where the socket gets mounted inside the container.
const SOCKET_PATH: &str = "/tmp/.X11-unix/X1";
const DISPLAY: &str = ":1";
const CONONICAL_DISPLAY_PATH: &str = "/tmp/.X11-unix";

pub struct DisplaySocket(u32);

impl DisplaySocket {
fn pick_unused() -> anyhow::Result<Self> {
use rustix::net::*;

// Because we're using a mount namespace, we don't need to worry about
// system sockets in /tmp leaking into our container. However, because we
// don't use a network namespace, it is possible for system abstract sockets
// to be available. We can check that this isn't the case by attempting to
// bind the abstract socket.
let mut display = 1;
let sock = socket(AddressFamily::UNIX, SocketType::STREAM, None)?;

loop {
// By convention, the name is the same as the path.
let dp = DisplaySocket(display);

match rustix::net::bind_unix(
&sock,
&SocketAddrUnix::new_abstract_name(dp.path().as_os_str().as_encoded_bytes())?,
) {
Ok(()) => return Ok(dp),
Err(e) if e.kind() == io::ErrorKind::AddrInUse => display += 1,
Err(e) => return Err(e.into()),
}
}
}

pub fn display(&self) -> String {
format!(":{}", self.0)
}

pub fn path(&self) -> PathBuf {
Path::new(CONONICAL_DISPLAY_PATH).join(format!("X{}", self.0))
}
}

impl XWayland {
pub fn spawn(
Expand All @@ -46,14 +84,17 @@ impl XWayland {
// it's ready.
let (displayfd_send, displayfd_recv) = mio::unix::pipe::new()?;

let display_socket = DisplaySocket::pick_unused()?;

// Put the socket in a folder, so we can bind-mount that to
// /tmp/.X11-unix inside the container.
let socket_path = xdg_runtime_dir
// /tmp/.X11-unix inside the (app) container.
let extern_socket_path = xdg_runtime_dir
.as_ref()
.join(Path::new(SOCKET_PATH).strip_prefix("/tmp").unwrap());
std::fs::create_dir_all(socket_path.parent().unwrap())?;
.join(display_socket.path().strip_prefix("/").unwrap());
let extern_socket_dir = extern_socket_path.parent().unwrap();
std::fs::create_dir_all(extern_socket_dir)?;

let socket = mio::net::UnixListener::bind(&socket_path)?;
let socket = mio::net::UnixListener::bind(&extern_socket_path)?;

let exe = find_executable_in_path("Xwayland")
.ok_or(anyhow!("Xwayland not in PATH"))?
Expand Down Expand Up @@ -97,7 +138,7 @@ impl XWayland {
}

let child = container.spawn().context("failed to spawn XWayland")?;
debug!(x11_socket = ?socket_path, "spawned Xwayland instance");
debug!(x11_socket = ?extern_socket_path, "spawned Xwayland instance");

// Insert the client into the display handle. The order is important
// here; XWayland never starts up at all unless it can roundtrip with
Expand All @@ -108,10 +149,11 @@ impl XWayland {
)?;

Ok(Self {
display_socket: socket_path,
display_socket,
displayfd_recv,
child,

extern_socket_dir: extern_socket_dir.to_owned(),
xwm_socket: Some(xwm_compositor),
})
}
Expand Down Expand Up @@ -140,11 +182,8 @@ impl XWayland {
}

pub fn prepare_socket(&self, container: &mut Container) {
container.bind_mount(
self.display_socket.parent().unwrap(),
Path::new(SOCKET_PATH).parent().unwrap(),
);
container.set_env("DISPLAY", DISPLAY);
container.bind_mount(&self.extern_socket_dir, Path::new(CONONICAL_DISPLAY_PATH));
container.set_env("DISPLAY", self.display_socket.display());
}
}

Expand Down
3 changes: 2 additions & 1 deletion mm-server/src/session/reactor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,7 +255,8 @@ impl Reactor {
if let Some(bug_report_dir) = bug_report_dir.as_ref() {
let p = bug_report_dir.to_owned();
let wayland_socket = socket_name.clone();
let x11_socket = xwayland.as_ref().map(|x| x.display_socket.clone());
let x11_socket = xwayland.as_ref().map(|x| x.display_socket.path().clone());

std::thread::spawn(move || {
save_glxinfo_eglinfo(
&p,
Expand Down

0 comments on commit 5c554cc

Please sign in to comment.