From 220605a5fd03d7181d55b59d6bd147c86a4c8bc0 Mon Sep 17 00:00:00 2001 From: Chirantan Ekbote Date: Thu, 14 Nov 2019 18:36:06 +0900 Subject: io_jail: Replace rlim_t with rlim64_t rlim_t is defined as an unsigned long but importantly, it is defined as what the _kernel_ thinks is an unsigned long. This means that when you have a 32-bit userspace and a 64-bit kernel (like we do for arm64 chromebooks), rlim_t is 64 bits. This isn't really a problem for C and C++ code because they use the headers from the kernel where rlim_t is properly sized but it doesn't really work for rust. The libc crate defines rlim_t as an alias for ::std::os::raw::c_ulong, which leads to the rust compiler thinking that it has a 32 bit width. Hilarity ensues when you attempt to cross the rust -> C FFI barrier with these conflicting definitions. The rust compiler thinks the parameters can fit in 32 bit registers so it puts the `cur` parameter in r2 and the `max` parameter in r3. On the other hand, the C code knows that the parameters are 64-bit values and combines r2/r3 to create the 64-bit `cur` value and uses the first 8 bytes on the stack as the `max` value. This leads to a `cur` value that is way too large and a nonsensical `max` value that depends on whatever happened to be on the stack at the time. Fix this by changing the library bindings to u64 and the Minijail::set_rlimit parameters to rlim64_t. Once we add a method to minijail that accepts rlim64_t's we can switch the library bindings to use that as well. BUG=b:136128319 TEST=`tast run vm.Virtiofs` on kevin Change-Id: I8f58923c4768ecfe827d2a5d73c72dc778fe419c Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform/crosvm/+/1916560 Reviewed-by: Chirantan Ekbote Tested-by: Chirantan Ekbote Tested-by: kokoro Commit-Queue: Chirantan Ekbote --- io_jail/src/lib.rs | 5 +++-- io_jail/src/libminijail.rs | 4 ++-- src/linux.rs | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/io_jail/src/lib.rs b/io_jail/src/lib.rs index 59d6887..82034a3 100644 --- a/io_jail/src/lib.rs +++ b/io_jail/src/lib.rs @@ -250,9 +250,10 @@ impl Minijail { pub fn set_rlimit( &mut self, kind: libc::c_int, - cur: libc::rlim_t, - max: libc::rlim_t, + cur: libc::rlim64_t, + max: libc::rlim64_t, ) -> Result<()> { + // TODO(chirantan): Switch to minijail_rlimit64 once that lands in libminijail. let errno = unsafe { libminijail::minijail_rlimit(self.jail, kind, cur, max) }; if errno == 0 { Ok(()) diff --git a/io_jail/src/libminijail.rs b/io_jail/src/libminijail.rs index 14175dc..7347f41 100644 --- a/io_jail/src/libminijail.rs +++ b/io_jail/src/libminijail.rs @@ -2,7 +2,7 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -use libc::{gid_t, pid_t, rlim_t, uid_t}; +use libc::{gid_t, pid_t, uid_t}; use std::os::raw::{c_char, c_int, c_long, c_ulong}; /// Struct minijail is an opaque type inside libminijail. @@ -19,7 +19,7 @@ extern "C" { pub fn minijail_keep_supplementary_gids(j: *mut minijail); pub fn minijail_change_user(j: *mut minijail, user: *const c_char) -> c_int; pub fn minijail_change_group(j: *mut minijail, group: *const c_char) -> c_int; - pub fn minijail_rlimit(j: *mut minijail, kind: c_int, cur: rlim_t, max: rlim_t) -> c_int; + pub fn minijail_rlimit(j: *mut minijail, kind: c_int, cur: u64, max: u64) -> c_int; pub fn minijail_use_seccomp(j: *mut minijail); pub fn minijail_no_new_privs(j: *mut minijail); pub fn minijail_use_seccomp_filter(j: *mut minijail); diff --git a/src/linux.rs b/src/linux.rs index bd6ee5c..ffdb829 100644 --- a/src/linux.rs +++ b/src/linux.rs @@ -287,7 +287,7 @@ impl AsRawFd for TaggedControlSocket { } } -fn get_max_open_files() -> Result { +fn get_max_open_files() -> Result { let mut buf = String::with_capacity(32); File::open("/proc/sys/fs/file-max") .and_then(|mut f| f.read_to_string(&mut buf)) -- cgit 1.4.1