From 713804208fa8cca001d6a415058d8e01ce1c59f4 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Tue, 9 Aug 2022 13:03:43 -0700 Subject: [PATCH 1/3] Stop returning `NOTCAPABLE` errors from WASI calls. `ENOTCAPABLE` was an error code that is used as part of the rights system, from CloudABI. There is a set of flags associated with each file descriptor listing which operations can be performed with the file descriptor, and if an attempt is made to perform an operation with a file descriptor that isn't permitted by its rights flags, it fails with `ENOTCAPABLE`. WASI is removing the rights system. For example, WebAssembly/wasi-libc#294 removed support for translating `ENOTCAPABLE` into POSIX error codes, on the assumption that engines should stop using it. So as another step to migrating away from the rights system, remove uses of the `ENOTCAPABLE` error. --- .../src/bin/path_open_read_without_rights.rs | 2 +- .../wasi-tests/src/bin/truncation_rights.rs | 2 +- crates/wasi-common/src/dir.rs | 15 ++++++++++++--- crates/wasi-common/src/error.rs | 9 ++++++--- crates/wasi-common/src/file.rs | 12 +++++++++++- crates/wasi-common/src/snapshots/preview_1.rs | 1 + 6 files changed, 32 insertions(+), 9 deletions(-) diff --git a/crates/test-programs/wasi-tests/src/bin/path_open_read_without_rights.rs b/crates/test-programs/wasi-tests/src/bin/path_open_read_without_rights.rs index c67fbe128087..0adceee9a861 100644 --- a/crates/test-programs/wasi-tests/src/bin/path_open_read_without_rights.rs +++ b/crates/test-programs/wasi-tests/src/bin/path_open_read_without_rights.rs @@ -30,7 +30,7 @@ unsafe fn try_read_file(dir_fd: wasi::Fd) { wasi::fd_read(fd, &[iovec]) .expect_err("reading bytes from file should fail") .raw_error(), - wasi::ERRNO_NOTCAPABLE + wasi::ERRNO_BADF ); } diff --git a/crates/test-programs/wasi-tests/src/bin/truncation_rights.rs b/crates/test-programs/wasi-tests/src/bin/truncation_rights.rs index c28d11f1fabe..5faa85fd91dd 100644 --- a/crates/test-programs/wasi-tests/src/bin/truncation_rights.rs +++ b/crates/test-programs/wasi-tests/src/bin/truncation_rights.rs @@ -68,7 +68,7 @@ unsafe fn test_truncation_rights(dir_fd: wasi::Fd) { wasi::path_open(dir_fd, 0, "file", wasi::OFLAGS_TRUNC, 0, 0, 0) .expect_err("truncating a file without path_filestat_set_size right") .raw_error(), - wasi::ERRNO_NOTCAPABLE + wasi::ERRNO_PERM ); } diff --git a/crates/wasi-common/src/dir.rs b/crates/wasi-common/src/dir.rs index 464f527325e4..318a087548d2 100644 --- a/crates/wasi-common/src/dir.rs +++ b/crates/wasi-common/src/dir.rs @@ -76,15 +76,24 @@ impl DirEntry { if self.caps.contains(caps) { Ok(()) } else { - Err(Error::not_capable().context(format!("desired {:?}, has {:?}", caps, self.caps,))) + let missing = caps & !self.caps; + if missing.intersects(DirCaps::READDIR) { + Err(Error::not_dir() + .context(format!("desired rights {:?}, has {:?}", caps, self.caps))) + } else { + Err(Error::perm() + .context(format!("desired rights {:?}, has {:?}", caps, self.caps))) + } } } pub fn capable_of_file(&self, caps: FileCaps) -> Result<(), Error> { if self.file_caps.contains(caps) { Ok(()) } else { - Err(Error::not_capable() - .context(format!("desired {:?}, has {:?}", caps, self.file_caps))) + Err(Error::perm().context(format!( + "desired rights {:?}, has {:?}", + caps, self.file_caps + ))) } } pub fn drop_caps_to(&mut self, caps: DirCaps, file_caps: FileCaps) -> Result<(), Error> { diff --git a/crates/wasi-common/src/error.rs b/crates/wasi-common/src/error.rs index e843a11766a9..0d50ab213c05 100644 --- a/crates/wasi-common/src/error.rs +++ b/crates/wasi-common/src/error.rs @@ -72,6 +72,9 @@ pub enum ErrorKind { /// Errno::Spipe: Invalid seek #[error("Spipe: Invalid seek")] Spipe, + /// Errno::Perm: Permission denied + #[error("Permission denied")] + Perm, /// Errno::NotCapable: Not capable #[error("Not capable")] NotCapable, @@ -92,7 +95,7 @@ pub trait ErrorExt { fn overflow() -> Self; fn range() -> Self; fn seek_pipe() -> Self; - fn not_capable() -> Self; + fn perm() -> Self; } impl ErrorExt for Error { @@ -138,7 +141,7 @@ impl ErrorExt for Error { fn seek_pipe() -> Self { ErrorKind::Spipe.into() } - fn not_capable() -> Self { - ErrorKind::NotCapable.into() + fn perm() -> Self { + ErrorKind::Perm.into() } } diff --git a/crates/wasi-common/src/file.rs b/crates/wasi-common/src/file.rs index 0de79e312852..5415769cff0f 100644 --- a/crates/wasi-common/src/file.rs +++ b/crates/wasi-common/src/file.rs @@ -238,7 +238,17 @@ impl FileEntry { if self.caps.contains(caps) { Ok(()) } else { - Err(Error::not_capable().context(format!("desired {:?}, has {:?}", caps, self.caps,))) + let missing = caps & !self.caps; + if missing.intersects(FileCaps::READ | FileCaps::WRITE) { + // `EBADF` is a little surprising here because it's also used + // for unknown-file-descriptor errors, but it's what POSIX uses + // in this situation. + Err(Error::badf() + .context(format!("desired rights {:?}, has {:?}", caps, self.caps))) + } else { + Err(Error::perm() + .context(format!("desired rights {:?}, has {:?}", caps, self.caps))) + } } } diff --git a/crates/wasi-common/src/snapshots/preview_1.rs b/crates/wasi-common/src/snapshots/preview_1.rs index 4eb7476a5160..6e2506480ca7 100644 --- a/crates/wasi-common/src/snapshots/preview_1.rs +++ b/crates/wasi-common/src/snapshots/preview_1.rs @@ -84,6 +84,7 @@ impl From for types::Errno { ErrorKind::Range => Errno::Range, ErrorKind::Spipe => Errno::Spipe, ErrorKind::NotCapable => Errno::Notcapable, + ErrorKind::Perm => Errno::Perm, } } } From d46e6d3dfaf1f5b1a3b31ed2106f795143040588 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 10 Aug 2022 12:01:03 -0700 Subject: [PATCH 2/3] Update crates/wasi-common/src/file.rs Co-authored-by: Jamey Sharp --- crates/wasi-common/src/file.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/wasi-common/src/file.rs b/crates/wasi-common/src/file.rs index 5415769cff0f..8d4bf6a45a61 100644 --- a/crates/wasi-common/src/file.rs +++ b/crates/wasi-common/src/file.rs @@ -239,16 +239,15 @@ impl FileEntry { Ok(()) } else { let missing = caps & !self.caps; - if missing.intersects(FileCaps::READ | FileCaps::WRITE) { + let err = if missing.intersects(FileCaps::READ | FileCaps::WRITE) { // `EBADF` is a little surprising here because it's also used // for unknown-file-descriptor errors, but it's what POSIX uses // in this situation. - Err(Error::badf() - .context(format!("desired rights {:?}, has {:?}", caps, self.caps))) + Error::badf() } else { - Err(Error::perm() - .context(format!("desired rights {:?}, has {:?}", caps, self.caps))) - } + Error::perm() + }; + Err(err.context(format!("desired rights {:?}, has {:?}", caps, self.caps))) } } From 65bc2245cbc2d367307f774768f9c0ea65be8085 Mon Sep 17 00:00:00 2001 From: Dan Gohman Date: Wed, 10 Aug 2022 12:01:10 -0700 Subject: [PATCH 3/3] Update crates/wasi-common/src/dir.rs Co-authored-by: Jamey Sharp --- crates/wasi-common/src/dir.rs | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/crates/wasi-common/src/dir.rs b/crates/wasi-common/src/dir.rs index 318a087548d2..18818763f515 100644 --- a/crates/wasi-common/src/dir.rs +++ b/crates/wasi-common/src/dir.rs @@ -77,13 +77,12 @@ impl DirEntry { Ok(()) } else { let missing = caps & !self.caps; - if missing.intersects(DirCaps::READDIR) { - Err(Error::not_dir() - .context(format!("desired rights {:?}, has {:?}", caps, self.caps))) + let err = if missing.intersects(DirCaps::READDIR) { + Error::not_dir() } else { - Err(Error::perm() - .context(format!("desired rights {:?}, has {:?}", caps, self.caps))) - } + Error::perm() + }; + Err(err.context(format!("desired rights {:?}, has {:?}", caps, self.caps))) } } pub fn capable_of_file(&self, caps: FileCaps) -> Result<(), Error> {