From 293c61cf11f0894bbdc9e8ee7a9ebaf5c667791e Mon Sep 17 00:00:00 2001 From: Chirantan Ekbote Date: Thu, 4 Jan 2018 16:19:17 -0800 Subject: Clean up wayland device jail The jail for the wayland device used chown to ensure that its jail had the proper permissions for the wayland socket to be bind mounted into it. This creates some unnecessary complexity because it requires careful management of the user and group and crosvm runs as (a non-root user cannot change the owner of a directory) or that crosvm has the CAP_CHOWN capability. Instead of trying to make the permissions fit, just have the jail mount a small tmpfs over the jail's root directory. This is one of the things that a process inside a user namespace has the ability to do. Bind mounting the wayland socket into this tmpfs then just works without any other issues. BUG=chromium:799523 TEST=linux vm boots with no errors Change-Id: Ic2240f430c7fd332a15b4fcd4e52374799eb6c9d Signed-off-by: Chirantan Ekbote Reviewed-on: https://chromium-review.googlesource.com/851413 Commit-Ready: Zach Reizner Tested-by: Zach Reizner Reviewed-by: Stephen Barber --- src/linux.rs | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) (limited to 'src/linux.rs') diff --git a/src/linux.rs b/src/linux.rs index fbd8e1c..46faed2 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -360,7 +360,6 @@ fn setup_mmio_bus(cfg: &Config, } } - let wl_root = TempDir::new(&PathBuf::from("/tmp/wl_root")).map_err(Error::WaylandTempDir)?; if let Some(wayland_socket_path) = cfg.wayland_socket_path.as_ref() { let jailed_wayland_path = Path::new("/wayland-0"); @@ -375,9 +374,15 @@ fn setup_mmio_bus(cfg: &Config, .map_err(Error::WaylandDeviceNew)?); let jail = if cfg.multiprocess { - let wl_root_path = wl_root.as_path().unwrap(); // Won't fail if new succeeded. let policy_path: PathBuf = cfg.seccomp_policy_dir.join("wl_device.policy"); - let mut jail = create_base_minijail(wl_root_path, &policy_path)?; + let mut jail = create_base_minijail(empty_root_path, &policy_path)?; + + // Create a tmpfs in the device's root directory so that we can bind mount the + // wayland socket into it. The size=67108864 is size=64*1024*1024 or size=64MB. + jail.mount_with_data(Path::new("none"), Path::new("/"), "tmpfs", + (libc::MS_NOSUID | libc::MS_NODEV | libc::MS_NOEXEC) as usize, + "size=67108864") + .unwrap(); // Bind mount the wayland socket into jail's root. This is necessary since each // new wayland context must open() the socket. @@ -399,13 +404,6 @@ fn setup_mmio_bus(cfg: &Config, geteuid() } }; - let crosvm_gid = match get_group_id(&crosvm_user_group) { - Ok(u) => u, - Err(e) => { - warn!("falling back to current group id for Wayland: {:?}", e); - getegid() - } - }; jail.change_uid(crosvm_uid); jail.change_gid(wayland_gid); jail.uidmap(&format!("{0} {0} 1", crosvm_uid)) @@ -413,12 +411,6 @@ fn setup_mmio_bus(cfg: &Config, jail.gidmap(&format!("{0} {0} 1", wayland_gid)) .map_err(Error::SettingGidMap)?; - // chown the root directory for the jail so we can actually bind mount the socket. - let wayland_root_cstr = CString::new(wl_root_path.as_os_str().to_str().unwrap()) - .unwrap(); - chown(&wayland_root_cstr, crosvm_uid, crosvm_gid) - .map_err(Error::ChownWaylandRoot)?; - Some(jail) } else { None -- cgit 1.4.1