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 <chirantan@chromium.org>
Tested-by: Chirantan Ekbote <chirantan@chromium.org>
Tested-by: kokoro <noreply+kokoro@google.com>
Commit-Queue: Chirantan Ekbote <chirantan@chromium.org>
This commit is contained in:
Chirantan Ekbote 2019-11-14 18:36:06 +09:00 committed by Commit Bot
parent 5277958078
commit 220605a5fd
3 changed files with 6 additions and 5 deletions

View file

@ -250,9 +250,10 @@ impl Minijail {
pub fn set_rlimit( pub fn set_rlimit(
&mut self, &mut self,
kind: libc::c_int, kind: libc::c_int,
cur: libc::rlim_t, cur: libc::rlim64_t,
max: libc::rlim_t, max: libc::rlim64_t,
) -> Result<()> { ) -> Result<()> {
// TODO(chirantan): Switch to minijail_rlimit64 once that lands in libminijail.
let errno = unsafe { libminijail::minijail_rlimit(self.jail, kind, cur, max) }; let errno = unsafe { libminijail::minijail_rlimit(self.jail, kind, cur, max) };
if errno == 0 { if errno == 0 {
Ok(()) Ok(())

View file

@ -2,7 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // 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}; use std::os::raw::{c_char, c_int, c_long, c_ulong};
/// Struct minijail is an opaque type inside libminijail. /// 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_keep_supplementary_gids(j: *mut minijail);
pub fn minijail_change_user(j: *mut minijail, user: *const c_char) -> c_int; 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_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_use_seccomp(j: *mut minijail);
pub fn minijail_no_new_privs(j: *mut minijail); pub fn minijail_no_new_privs(j: *mut minijail);
pub fn minijail_use_seccomp_filter(j: *mut minijail); pub fn minijail_use_seccomp_filter(j: *mut minijail);

View file

@ -287,7 +287,7 @@ impl AsRawFd for TaggedControlSocket {
} }
} }
fn get_max_open_files() -> Result<libc::rlim_t> { fn get_max_open_files() -> Result<libc::rlim64_t> {
let mut buf = String::with_capacity(32); let mut buf = String::with_capacity(32);
File::open("/proc/sys/fs/file-max") File::open("/proc/sys/fs/file-max")
.and_then(|mut f| f.read_to_string(&mut buf)) .and_then(|mut f| f.read_to_string(&mut buf))