fuchsia: remove use of replace_as_executable (#16690)

On Fuchsia, we can now get executable VMOs from trusted backing
filesystems.  This allows us to remove the use of replace_as_executable
in favor of opening files with `fdio_open_fd_at` with the
`OPEN_RIGHT_EXECUTABLE` flag and getting VMOs by calling
`fdio_get_vmo_exec`.

By moving the responsibility for executability into the filesystem, we
should be able to remove deprecated-ambient-replace-as-executable from
component manifests for non-JIT runners (the JIT runners still call
replace_as_executable in Dart's allocator).

Test: verified locally that this works on Astro on a _user build with
the runtime allowlist tightened.
This commit is contained in:
Drew Fisher 2020-02-25 13:24:07 -08:00 committed by GitHub
parent 468b371ff4
commit ff921cd608
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 50 additions and 45 deletions

View File

@ -3,9 +3,6 @@
"data": "data/goodbye_dart_aot"
},
"sandbox": {
"features": [
"deprecated-ambient-replace-as-executable"
],
"services": [
"fuchsia.intl.PropertyProvider",
"fuchsia.sys.Environment"

View File

@ -5,7 +5,6 @@
"sandbox": {
"features": [
"config-data",
"deprecated-ambient-replace-as-executable",
"root-ssl-certificates"
],
"services": [

View File

@ -5,7 +5,6 @@
"sandbox": {
"features": [
"config-data",
"deprecated-ambient-replace-as-executable",
"root-ssl-certificates"
],
"services": [

View File

@ -431,7 +431,8 @@ class FileInNamespaceBuffer final : public fml::Mapping {
FileInNamespaceBuffer(int namespace_fd, const char* path, bool executable)
: address_(nullptr), size_(0) {
fuchsia::mem::Buffer buffer;
if (!dart_utils::VmoFromFilenameAt(namespace_fd, path, &buffer)) {
if (!dart_utils::VmoFromFilenameAt(namespace_fd, path, executable,
&buffer)) {
return;
}
if (buffer.size == 0) {
@ -441,17 +442,6 @@ class FileInNamespaceBuffer final : public fml::Mapping {
uint32_t flags = ZX_VM_PERM_READ;
if (executable) {
flags |= ZX_VM_PERM_EXECUTE;
// VmoFromFilenameAt will return VMOs without ZX_RIGHT_EXECUTE,
// so we need replace_as_executable to be able to map them as
// ZX_VM_PERM_EXECUTE.
// TODO(mdempsky): Update comment once SEC-42 is fixed.
zx_status_t status =
buffer.vmo.replace_as_executable(zx::handle(), &buffer.vmo);
if (status != ZX_OK) {
FML_LOG(FATAL) << "Failed to make VMO executable: "
<< zx_status_get_string(status);
}
}
uintptr_t addr;
zx_status_t status =

View File

@ -5,7 +5,6 @@
"sandbox": {
"features": [
"config-data",
"deprecated-ambient-replace-as-executable",
"root-ssl-certificates",
"vulkan"
],

View File

@ -5,7 +5,6 @@
"sandbox": {
"features": [
"config-data",
"deprecated-ambient-replace-as-executable",
"root-ssl-certificates",
"vulkan"
],

View File

@ -87,7 +87,7 @@ bool InitializeICU() {
const char* data_path = kIcuDataPath;
fuchsia::mem::Buffer icu_data;
if (!dart_utils::VmoFromFilename(data_path, &icu_data)) {
if (!dart_utils::VmoFromFilename(data_path, false, &icu_data)) {
return false;
}

View File

@ -36,7 +36,7 @@ static bool OpenVmo(fuchsia::mem::Buffer* resource_vmo,
dart_utils::Check(path[0] != '/', LOG_TAG);
if (namespc == nullptr) {
if (!VmoFromFilename(path, resource_vmo)) {
if (!VmoFromFilename(path, executable, resource_vmo)) {
return false;
}
} else {
@ -46,27 +46,14 @@ static bool OpenVmo(fuchsia::mem::Buffer* resource_vmo,
return false;
}
bool result = dart_utils::VmoFromFilenameAt(root_dir, path, resource_vmo);
bool result =
dart_utils::VmoFromFilenameAt(root_dir, path, executable, resource_vmo);
close(root_dir);
if (!result) {
return result;
}
}
if (executable) {
// VmoFromFilenameAt will return VMOs without ZX_RIGHT_EXECUTE,
// so we need replace_as_executable to be able to map them as
// ZX_VM_PERM_EXECUTE.
// TODO(mdempsky): Update comment once SEC-42 is fixed.
zx_status_t status = resource_vmo->vmo.replace_as_executable(
zx::handle(), &resource_vmo->vmo);
if (status != ZX_OK) {
FX_LOGF(ERROR, LOG_TAG, "Failed to make VMO executable: %s",
zx_status_get_string(status));
return false;
}
}
return true;
}

View File

@ -7,15 +7,18 @@
#include <fcntl.h>
#include <sys/stat.h>
#include <fuchsia/io/cpp/fidl.h>
#include <fuchsia/mem/cpp/fidl.h>
#include <lib/fdio/directory.h>
#include <lib/fdio/io.h>
#include <lib/syslog/global.h>
#include <zircon/status.h>
#include "runtime/dart/utils/logging.h"
namespace {
bool VmoFromFd(int fd, fuchsia::mem::Buffer* buffer) {
bool VmoFromFd(int fd, bool executable, fuchsia::mem::Buffer* buffer) {
if (!buffer) {
FX_LOG(FATAL, LOG_TAG, "Invalid buffer pointer");
}
@ -27,7 +30,14 @@ bool VmoFromFd(int fd, fuchsia::mem::Buffer* buffer) {
}
zx_handle_t result = ZX_HANDLE_INVALID;
if (fdio_get_vmo_copy(fd, &result) != ZX_OK) {
zx_status_t status;
if (executable) {
status = fdio_get_vmo_exec(fd, &result);
} else {
status = fdio_get_vmo_copy(fd, &result);
}
if (status != ZX_OK) {
return false;
}
@ -42,20 +52,42 @@ bool VmoFromFd(int fd, fuchsia::mem::Buffer* buffer) {
namespace dart_utils {
bool VmoFromFilename(const std::string& filename,
bool executable,
fuchsia::mem::Buffer* buffer) {
return VmoFromFilenameAt(AT_FDCWD, filename, buffer);
// Note: the implementation here cannot be shared with VmoFromFilenameAt
// because fdio_open_fd_at does not aim to provide POSIX compatibility, and
// thus does not handle AT_FDCWD as dirfd.
uint32_t flags = fuchsia::io::OPEN_RIGHT_READABLE |
(executable ? fuchsia::io::OPEN_RIGHT_EXECUTABLE : 0);
zx_status_t status;
int fd;
status = fdio_open_fd(filename.c_str(), flags, &fd);
if (status != ZX_OK) {
FX_LOGF(ERROR, LOG_TAG, "fdio_open_fd(\"%s\", %08x) failed: %s",
filename.c_str(), flags, zx_status_get_string(status));
return false;
}
bool result = VmoFromFd(fd, executable, buffer);
close(fd);
return result;
}
bool VmoFromFilenameAt(int dirfd,
const std::string& filename,
bool executable,
fuchsia::mem::Buffer* buffer) {
int fd = openat(dirfd, filename.c_str(), O_RDONLY);
if (fd == -1) {
FX_LOGF(ERROR, LOG_TAG, "openat(\"%s\") failed: %s", filename.c_str(),
strerror(errno));
uint32_t flags = fuchsia::io::OPEN_RIGHT_READABLE |
(executable ? fuchsia::io::OPEN_RIGHT_EXECUTABLE : 0);
zx_status_t status;
int fd;
status = fdio_open_fd_at(dirfd, filename.c_str(), flags, &fd);
if (status != ZX_OK) {
FX_LOGF(ERROR, LOG_TAG, "fdio_open_fd_at(%d, \"%s\", %08x) failed: %s",
dirfd, filename.c_str(), flags, zx_status_get_string(status));
return false;
}
bool result = VmoFromFd(fd, buffer);
bool result = VmoFromFd(fd, executable, buffer);
close(fd);
return result;
}

View File

@ -11,10 +11,13 @@
namespace dart_utils {
bool VmoFromFilename(const std::string& filename, fuchsia::mem::Buffer* buffer);
bool VmoFromFilename(const std::string& filename,
bool executable,
fuchsia::mem::Buffer* buffer);
bool VmoFromFilenameAt(int dirfd,
const std::string& filename,
bool executable,
fuchsia::mem::Buffer* buffer);
zx_status_t IsSizeValid(const fuchsia::mem::Buffer& buffer, bool* is_valid);