Skip to content

Commit

Permalink
Fix stats where st_dev is negative (#329)
Browse files Browse the repository at this point in the history
* Fix stats where `st_dev` is negative

This is intended to be a fix for bytecodealliance/wasmtime#6978 where
some platforms use a signed integer for the `dev_t` type and it looks
like some `st_dev` fields can indeed be negative.

* Move lint location

* Comment odd location of `#[allow]`

* Fix a warning on beta Rust

* Fix a failing tempfile test

Bisection points at bytecodealliance/rustix#787 as other flags are
coming into the `Mode` enum which means that the `mode.bits()` is
`0o100644` which wasn't being tested for. The unknown bits are masked
off now.
  • Loading branch information
alexcrichton authored Sep 14, 2023
1 parent 963eebf commit 5e9248c
Show file tree
Hide file tree
Showing 4 changed files with 18 additions and 6 deletions.
16 changes: 15 additions & 1 deletion cap-primitives/src/rustix/fs/metadata_ext.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@ impl MetadataExt {

/// Constructs a new instance of `Metadata` from the given `Stat`.
#[inline]
#[allow(unused_comparisons)] // NB: rust-lang/rust#115823 requires this here instead of on `st_dev` processing below
pub(crate) fn from_rustix(stat: Stat) -> Metadata {
Metadata {
file_type: ImplFileTypeExt::from_raw_mode(stat.st_mode as RawMode),
Expand Down Expand Up @@ -164,7 +165,20 @@ impl MetadataExt {
created: None,

ext: Self {
dev: u64::try_from(stat.st_dev).unwrap(),
// The type of `st_dev` is `dev_t` which is signed on some
// platforms and unsigned on other platforms. A `u64` is enough
// to work for all unsigned platforms, and for signed platforms
// perform a sign extension to `i64` and then view that as an
// unsigned 64-bit number instead.
//
// Note that the `unused_comparisons` is ignored here for
// platforms where it's unsigned since the first branch here
// will never be taken.
dev: if stat.st_dev < 0 {
i64::try_from(stat.st_dev).unwrap() as u64
} else {
u64::try_from(stat.st_dev).unwrap()
},
ino: stat.st_ino.into(),
#[cfg(not(target_os = "wasi"))]
mode: u32::from(stat.st_mode),
Expand Down
2 changes: 1 addition & 1 deletion cap-tempfile/src/tempfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ mod test {
let metadata = tf.as_file().metadata().unwrap();
let mode = metadata.mode();
let mode = Mode::from_bits_truncate(mode);
assert_eq!(0o666 & !umask, mode.bits());
assert_eq!(0o666 & !umask, mode.bits() & 0o777);
}
// And that we can write
tf.write_all(b"hello world")?;
Expand Down
3 changes: 1 addition & 2 deletions tests/fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -677,7 +677,6 @@ fn recursive_rmdir_toctou() {
// deleted.
let tmpdir = tmpdir();
let victim_del_path = "victim_del";
let victim_del_path_clone = victim_del_path.clone();

// setup dest
let attack_dest_dir = "attack_dest";
Expand All @@ -695,7 +694,7 @@ fn recursive_rmdir_toctou() {
let tmpdir_clone = tmpdir.try_clone().unwrap();
let _t = thread::spawn(move || {
while drop_canary_weak.upgrade().is_some() {
let _ = tmpdir_clone.remove_dir_all(victim_del_path_clone);
let _ = tmpdir_clone.remove_dir_all(victim_del_path);
}
});

Expand Down
3 changes: 1 addition & 2 deletions tests/fs_utf8.rs
Original file line number Diff line number Diff line change
Expand Up @@ -680,7 +680,6 @@ fn recursive_rmdir_toctou() {
// deleted.
let tmpdir = tmpdir();
let victim_del_path = "victim_del";
let victim_del_path_clone = victim_del_path.clone();

// setup dest
let attack_dest_dir = "attack_dest";
Expand All @@ -698,7 +697,7 @@ fn recursive_rmdir_toctou() {
let tmpdir_clone = tmpdir.try_clone().unwrap();
let _t = thread::spawn(move || {
while drop_canary_weak.upgrade().is_some() {
let _ = tmpdir_clone.remove_dir_all(victim_del_path_clone);
let _ = tmpdir_clone.remove_dir_all(victim_del_path);
}
});

Expand Down

0 comments on commit 5e9248c

Please sign in to comment.