vm_tools: p9: Use MAPPED_FLAGS for P9_{CREATE,EXCL,TRUNC,APPEND}
The rust standard library fails with InvalidInput for any call to open
where create[_new] is set, but write is not set.
https://rust-lang.github.io/rfcs/1252-open-options.html#combination-of-access-modes-and-creation-modes
This fails for some clients such as Microsoft Edge. It creates an empty
file first as O_RDONLY before reopening as O_RDWR to download a file.
Rather than call `.create`, `.create_new`, `.truncate`, or `.append`, in
lopen and lcreate, we have added P9_CREATE, P9_EXCL, P9_TRUNC, and
P9_APPEND to MAPPED_FLAGS, and we unconditionally set O_CREAT|O_EXCL for
lcreate.
BUG=b:161856673
TEST=unit tests
Change-Id: I010d91fa4b14ebb911f26df3af347a2be40e885d
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2324089
Tested-by: Joel Hockey <joelhockey@chromium.org>
Reviewed-by: Chirantan Ekbote <chirantan@chromium.org>
Commit-Queue: Joel Hockey <joelhockey@chromium.org>
Auto-Submit: Joel Hockey <joelhockey@chromium.org>
diff --git a/vm_tools/p9/src/server/mod.rs b/vm_tools/p9/src/server/mod.rs
index 0d3038b..640b080 100644
--- a/vm_tools/p9/src/server/mod.rs
+++ b/vm_tools/p9/src/server/mod.rs
@@ -39,8 +39,12 @@
const P9_SYNC: u32 = 0o04000000;
// Mapping from 9P flags to libc flags.
-const MAPPED_FLAGS: [(u32, i32); 10] = [
+const MAPPED_FLAGS: [(u32, i32); 14] = [
+ (P9_CREATE, libc::O_CREAT),
+ (P9_EXCL, libc::O_EXCL),
(P9_NOCTTY, libc::O_NOCTTY),
+ (P9_TRUNC, libc::O_TRUNC),
+ (P9_APPEND, libc::O_APPEND),
(P9_NONBLOCK, libc::O_NONBLOCK),
(P9_DSYNC, libc::O_DSYNC),
(P9_FASYNC, 0), // Unsupported
@@ -528,13 +532,10 @@
}
}
+ // MAPPED_FLAGS will handle append, create[_new], and truncate.
let file = fs::OpenOptions::new()
.read((lopen.flags & P9_NOACCESS) == 0 || (lopen.flags & P9_RDWR) != 0)
.write((lopen.flags & P9_WRONLY) != 0 || (lopen.flags & P9_RDWR) != 0)
- .append((lopen.flags & P9_APPEND) != 0)
- .truncate((lopen.flags & P9_TRUNC) != 0)
- .create((lopen.flags & P9_CREATE) != 0)
- .create_new((lopen.flags & P9_CREATE) != 0 && (lopen.flags & P9_EXCL) != 0)
.custom_flags(custom_flags)
.open(&fid.path)?;
@@ -567,14 +568,11 @@
}
}
- // The file must not already exist, which is why we unconditionally set
- // `create_new(true)`. This is also why we don't set `truncate`. If
- // the file does not exist then it doesn't need to be truncated.
+ // Set O_CREAT|O_EXCL, MAPPED_FLAGS will handle append and truncate.
+ custom_flags |= libc::O_CREAT | libc::O_EXCL;
let file = fs::OpenOptions::new()
.read((lcreate.flags & P9_NOACCESS) == 0 || (lcreate.flags & P9_RDWR) != 0)
.write((lcreate.flags & P9_WRONLY) != 0 || (lcreate.flags & P9_RDWR) != 0)
- .append((lcreate.flags & P9_APPEND) != 0)
- .create_new(true)
.custom_flags(custom_flags)
.mode(lcreate.mode & 0o755)
.open(&path)?;
diff --git a/vm_tools/p9/src/server/tests.rs b/vm_tools/p9/src/server/tests.rs
index 15d569e..56bec39 100644
--- a/vm_tools/p9/src/server/tests.rs
+++ b/vm_tools/p9/src/server/tests.rs
@@ -919,7 +919,7 @@
}
// Check that we can write to the file.
- if $flags & P9_RDWR != 0 || $flags & P9_WRONLY != 0 || $flags & P9_APPEND != 0 {
+ if $flags & P9_RDWR != 0 || $flags & P9_WRONLY != 0 {
write(&mut server, &test_dir, name, fid, $flags);
}
@@ -950,11 +950,7 @@
open_test!(read_write_file_open, P9_RDWR);
open_test!(write_only_file_open, P9_WRONLY);
-open_test!(
- create_read_only_file_open,
- P9_CREATE | _P9_RDONLY,
- io::ErrorKind::InvalidInput
-);
+open_test!(create_read_only_file_open, P9_CREATE | _P9_RDONLY);
open_test!(create_read_write_file_open, P9_CREATE | P9_RDWR);
open_test!(create_write_only_file_open, P9_CREATE | P9_WRONLY);
@@ -962,11 +958,7 @@
open_test!(append_read_write_file_open, P9_APPEND | P9_RDWR);
open_test!(append_write_only_file_open, P9_APPEND | P9_WRONLY);
-open_test!(
- trunc_read_only_file_open,
- P9_TRUNC | _P9_RDONLY,
- io::ErrorKind::InvalidInput
-);
+open_test!(trunc_read_only_file_open, P9_TRUNC | _P9_RDONLY);
open_test!(trunc_read_write_file_open, P9_TRUNC | P9_RDWR);
open_test!(trunc_write_only_file_open, P9_TRUNC | P9_WRONLY);
@@ -985,8 +977,7 @@
open_test!(
create_trunc_read_only_file_open,
- P9_CREATE | P9_TRUNC | _P9_RDONLY,
- io::ErrorKind::InvalidInput
+ P9_CREATE | P9_TRUNC | _P9_RDONLY
);
open_test!(
create_trunc_read_write_file_open,
@@ -999,40 +990,34 @@
open_test!(
append_trunc_read_only_file_open,
- P9_APPEND | P9_TRUNC | _P9_RDONLY,
- io::ErrorKind::InvalidInput
+ P9_APPEND | P9_TRUNC | _P9_RDONLY
);
open_test!(
append_trunc_read_write_file_open,
- P9_APPEND | P9_TRUNC | P9_RDWR,
- io::ErrorKind::InvalidInput
+ P9_APPEND | P9_TRUNC | P9_RDWR
);
open_test!(
append_trunc_wronly_file_open,
- P9_APPEND | P9_TRUNC | P9_WRONLY,
- io::ErrorKind::InvalidInput
+ P9_APPEND | P9_TRUNC | P9_WRONLY
);
open_test!(
create_append_trunc_read_only_file_open,
- P9_CREATE | P9_APPEND | P9_TRUNC | _P9_RDONLY,
- io::ErrorKind::InvalidInput
+ P9_CREATE | P9_APPEND | P9_TRUNC | _P9_RDONLY
);
open_test!(
create_append_trunc_read_write_file_open,
- P9_CREATE | P9_APPEND | P9_TRUNC | P9_RDWR,
- io::ErrorKind::InvalidInput
+ P9_CREATE | P9_APPEND | P9_TRUNC | P9_RDWR
);
open_test!(
create_append_trunc_wronly_file_open,
- P9_CREATE | P9_APPEND | P9_TRUNC | P9_WRONLY,
- io::ErrorKind::InvalidInput
+ P9_CREATE | P9_APPEND | P9_TRUNC | P9_WRONLY
);
open_test!(
create_excl_read_only_file_open,
P9_CREATE | P9_EXCL | _P9_RDONLY,
- io::ErrorKind::InvalidInput
+ io::ErrorKind::AlreadyExists
);
open_test!(
create_excl_read_write_file_open,
@@ -1063,7 +1048,7 @@
check_attr(&mut server, fid, &md);
// Check that we can write to the file.
- if $flags & P9_RDWR != 0 || $flags & P9_WRONLY != 0 || $flags & P9_APPEND != 0 {
+ if $flags & P9_RDWR != 0 || $flags & P9_WRONLY != 0 {
write(&mut server, &test_dir, name, fid, $flags);
}
@@ -1089,12 +1074,7 @@
};
}
-create_test!(
- read_only_file_create,
- _P9_RDONLY,
- 0o600u32,
- io::ErrorKind::InvalidInput
-);
+create_test!(read_only_file_create, _P9_RDONLY, 0o600u32);
create_test!(read_write_file_create, P9_RDWR, 0o600u32);
create_test!(write_only_file_create, P9_WRONLY, 0o600u32);
