From 238122258a2d5cb9324ab035f62bc03aa6bc7bdc Mon Sep 17 00:00:00 2001 From: Roland McGrath Date: Thu, 23 May 2024 19:16:09 +0000 Subject: [PATCH] [ld] ld::RemoteZygote This implements the "ELF-only zygote model". After using the ld::RemoteDynamicLinker API to launch a prototype process, it can be distilled into a simpler ld::RemoteZygote object that can be used to launch more identical new processes. Bug: 326524302 Change-Id: Ie46c239235ad2e2040227cf49ded133761cf57bd Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/1013912 Fuchsia-Auto-Submit: Roland McGrath Reviewed-by: Caslyn Tonelli Commit-Queue: Auto-Submit --- sdk/lib/ld/BUILD.gn | 1 + sdk/lib/ld/include/lib/ld/remote-abi-heap.h | 14 +- sdk/lib/ld/include/lib/ld/remote-abi.h | 6 +- .../ld/include/lib/ld/remote-dynamic-linker.h | 46 ++- sdk/lib/ld/include/lib/ld/remote-zygote.h | 378 ++++++++++++++++++ sdk/lib/ld/test/BUILD.gn | 6 +- .../test/ld-load-zircon-process-tests-base.cc | 10 +- .../test/ld-load-zircon-process-tests-base.h | 1 + sdk/lib/ld/test/ld-remote-process-tests.h | 6 + sdk/lib/ld/test/modules/BUILD.gn | 44 ++ sdk/lib/ld/test/modules/zygote-dep.cc | 9 + sdk/lib/ld/test/modules/zygote-secondary.cc | 19 + sdk/lib/ld/test/modules/zygote.cc | 26 ++ sdk/lib/ld/test/modules/zygote.h | 19 + sdk/lib/ld/test/modules/zygote.ifs | 8 + sdk/lib/ld/test/remote-tests.cc | 163 ++++++++ .../include/lib/elfldltl/segment-with-vmo.h | 78 ++-- 17 files changed, 775 insertions(+), 59 deletions(-) create mode 100644 sdk/lib/ld/include/lib/ld/remote-zygote.h create mode 100644 sdk/lib/ld/test/modules/zygote-dep.cc create mode 100644 sdk/lib/ld/test/modules/zygote-secondary.cc create mode 100644 sdk/lib/ld/test/modules/zygote.cc create mode 100644 sdk/lib/ld/test/modules/zygote.h create mode 100644 sdk/lib/ld/test/modules/zygote.ifs diff --git a/sdk/lib/ld/BUILD.gn b/sdk/lib/ld/BUILD.gn index 7b553b9a4dff..e1d48cfbcdf1 100644 --- a/sdk/lib/ld/BUILD.gn +++ b/sdk/lib/ld/BUILD.gn @@ -38,6 +38,7 @@ library_headers("headers") { "lib/ld/remote-decoded-module.h", "lib/ld/remote-dynamic-linker.h", "lib/ld/remote-load-module.h", + "lib/ld/remote-zygote.h", ] public_deps += [ diff --git a/sdk/lib/ld/include/lib/ld/remote-abi-heap.h b/sdk/lib/ld/include/lib/ld/remote-abi-heap.h index 5f3659d8e36c..f5f412798792 100644 --- a/sdk/lib/ld/include/lib/ld/remote-abi-heap.h +++ b/sdk/lib/ld/include/lib/ld/remote-abi-heap.h @@ -196,9 +196,10 @@ class RemoteAbiHeapLayout { // RemoteAbiHeapLayout constructor from RemoteAbiStub::data_size() and a // mutable RemoteLoadModule describing the same stub dynamic linker ELF file // presented to RemoteAbiStub::Init. -template +template class RemoteAbiHeap { public: + using Elf = typename RemoteModule::Elf; using size_type = typename Elf::size_type; using Addr = typename Elf::Addr; using Phdr = typename Elf::Phdr; @@ -219,8 +220,7 @@ class RemoteAbiHeap { // of the RemoteAbiHeap object. template static zx::result Create(Diagnostics& diagnostics, size_type stub_data_size, - RemoteLoadModule& stub_module, - RemoteAbiHeapLayout layout) { + RemoteModule& stub_module, RemoteAbiHeapLayout layout) { if (!stub_module.PrepareLoadInfo(diagnostics)) [[unlikely]] { return zx::error{ZX_ERR_NO_MEMORY}; } @@ -246,7 +246,7 @@ class RemoteAbiHeap { auto& new_segment = std::get(stub_module.load_info().segments().back()); // Build the RemoteAbiHeap. - RemoteAbiHeap heap; + RemoteAbiHeap heap; heap.strtab_ = strtab.offset_; heap.size_bytes_ = memsz; @@ -278,7 +278,7 @@ class RemoteAbiHeap { // // This should only be called on a stub_module that was previously modified // by a successful Create() call, and has since had its load_bias() set. - static size_type HeapVaddr(const RemoteLoadModule& stub_module) { + static size_type HeapVaddr(const RemoteModule& stub_module) { const auto& segment = stub_module.load_info().segments().back(); const size_type segment_vaddr = std::visit([](const auto& segment) { return segment.vaddr(); }, segment); @@ -366,7 +366,7 @@ class RemoteAbiHeap { } private: - using LoadInfo = typename RemoteLoadModule::LoadInfo; + using LoadInfo = typename RemoteModule::LoadInfo; using StubSegment = typename LoadInfo::Segment; using StubConstantSegment = typename LoadInfo::ConstantSegment; @@ -398,7 +398,7 @@ class RemoteAbiHeap { size_t segment_size) { assert(file_vmo); - const size_type page_size = RemoteLoadModule::Loader::page_size(); + const size_type page_size = RemoteModule::Loader::page_size(); const size_type filesz = old_segment.filesz(); const size_type memsz = (segment_size + page_size - 1) & -page_size; diff --git a/sdk/lib/ld/include/lib/ld/remote-abi.h b/sdk/lib/ld/include/lib/ld/remote-abi.h index 84fe29071298..79296a37b904 100644 --- a/sdk/lib/ld/include/lib/ld/remote-abi.h +++ b/sdk/lib/ld/include/lib/ld/remote-abi.h @@ -28,9 +28,10 @@ namespace ld { // before laying out the remote address space; Finish is called after address // space layout is known, to finalize the data. -template > +template class RemoteAbi { public: + using Elf = typename RemoteModule::Elf; using AbiStubPtr = typename RemoteAbiStub::Ptr; using size_type = typename Elf::size_type; @@ -38,7 +39,6 @@ class RemoteAbi { using TlsLayout = elfldltl::TlsLayout; using AbiStub = RemoteAbiStub; - using RemoteModule = RemoteLoadModule; using ModuleList = typename RemoteModule::List; using LocalAbi = abi::Abi; @@ -171,7 +171,7 @@ class RemoteAbi { } private: - using AbiHeap = RemoteAbiHeap; + using AbiHeap = RemoteAbiHeap; using AbiStringPtr = typename AbiHeap::template AbiPtr; using Abi = abi::Abi; diff --git a/sdk/lib/ld/include/lib/ld/remote-dynamic-linker.h b/sdk/lib/ld/include/lib/ld/remote-dynamic-linker.h index b75bb40864ec..ecc24d8c33ea 100644 --- a/sdk/lib/ld/include/lib/ld/remote-dynamic-linker.h +++ b/sdk/lib/ld/include/lib/ld/remote-dynamic-linker.h @@ -22,7 +22,11 @@ namespace ld { // It may or may not be the first or only dynamic linking session performed on // the same process. Each dynamic linking session defines its own symbolic // dynamic linking domain and has its own passive ABI (stub dynamic linker). -// TODO(https://fxbug.dev/326524302): Describe Zygote options. +// +// The second optional template parameter can select the "zygote mode" +// implementation. This is used by the API, which +// provides the ld::RemoteZygote::Linker alias. The zygote-mode linker is used +// in the same ways as the plain ld::RemoteDynamicLinker described here. // // Before creating an ld::RemoteDynamicLinker, the ld::RemoteAbiStub must be // provided (see ). Only a single ld::RemoteAbiStub @@ -131,8 +135,9 @@ namespace ld { // The VMAR handles are no longer available and the VmarLoader objects would // need to be reinitialized to be used again. The segment VMO handles are // still available, but when not in zygote mode they are in use by process -// mappings and must not be touched. TODO(https://fxbug.dev/326524302): In -// Zygote mode, it can also be distilled into a zygote. +// mappings and must not be touched. In zygote mode, the relocated segment +// VMOs will be made read-only and then reused (directly for RELRO mappings or +// via copy-on-write copies) to load additional as-relocated process images. // // Various other methods are provided for interrogating the list of modules and // accessing the dynamic linker stub module and the ld::RemoteAbi object. @@ -326,8 +331,8 @@ class RemoteDynamicLinker { // Other accessors should be used only after a successful Init call (below). - RemoteAbi& remote_abi() { return remote_abi_; } - const RemoteAbi& remote_abi() const { return remote_abi_; } + RemoteAbi& remote_abi() { return remote_abi_; } + const RemoteAbi& remote_abi() const { return remote_abi_; } List& modules() { return modules_; } const List& modules() const { return modules_; } @@ -702,7 +707,28 @@ class RemoteDynamicLinker { // Resolve against the successfully decoded modules, ignoring the others. return module.Relocate(diag, valid_modules, tls_desc_resolver); }; - return OnModules(valid_modules, relocate) && FinishAbi(diag); + + // After the segments are complete, make sure all the VMO handles are + // read-only so they don't accidentally get mutated. This isn't necessary + // in non-zygote mode since the object won't usually be saved long anyway. + auto protect_segments = [&diag](auto& module) -> bool { + return module.load_info().VisitSegments([&diag](auto& segment) -> bool { + using SegmentType = std::decay_t; + if constexpr (elfldltl::kSegmentHasFilesz) { + zx::result<> result = segment.MakeImmutable(); + if (result.is_error()) [[unlikely]] { + return diag.SystemError( // + "cannot drop ZX_RIGHT_WRITE from finished zygote VMO", + elfldltl::ZirconError{result.error_value()}); + } + } + return true; + }); + }; + + return OnModules(valid_modules, relocate) && FinishAbi(diag) && + (Zygote == RemoteLoadZygote::kNo || // No need for non-zygote. + OnModules(valid_modules, protect_segments)); } // Load each module into the VMARs created by Allocate. This should only be @@ -734,7 +760,11 @@ class RemoteDynamicLinker { // mappings and must not be disturbed. The VmarLoader object for each module // will be in moved-from state, and cannot be used without reinitialization. // - // TODO(https://fxbug.dev/326524302): Describe Zygote options. + // In zygote mode, this object can be moved into the ld::RemoteZygote + // constructor after Commit(). If it's moved in without Commit(), then all + // the mappings made in the original process VMAR will be destroyed and the + // existing process should not be started, but the zygote will still work + // just the same to start more processes. void Commit() { for (Module& module : ValidModules()) { // After this, destroying the module won't destroy its VMAR any more. No @@ -849,7 +879,7 @@ class RemoteDynamicLinker { } AbiStubPtr abi_stub_; - RemoteAbi remote_abi_; + RemoteAbi remote_abi_; List modules_; size_type max_tls_modid_ = 0; uint32_t stub_modid_ = 0; diff --git a/sdk/lib/ld/include/lib/ld/remote-zygote.h b/sdk/lib/ld/include/lib/ld/remote-zygote.h new file mode 100644 index 000000000000..7152afe02338 --- /dev/null +++ b/sdk/lib/ld/include/lib/ld/remote-zygote.h @@ -0,0 +1,378 @@ +// Copyright 2024 The Fuchsia Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef LIB_LD_REMOTE_ZYGOTE_H_ +#define LIB_LD_REMOTE_ZYGOTE_H_ + +#include + +#include "remote-dynamic-linker.h" + +namespace ld { + +// ld:RemoteZygote distills the work done by a zygote-mode dynamic linking +// session into simpler state that can be used to stamp out the same set of +// fully-relocated modules at the same addresses in new processes repeatedly. +// +// ld:RemoteZygote::Linker is an alias for zygote-mode ld::RemoteDynamicLinker, +// as defined in . That object performs the +// dynamic linking for the first process, using the VMAR provided to the +// Linker::Allocate() method to have the system choose load addresses with +// ASLR. Linker::Commit() should be completed before the Linker is moved into +// the zygote using ld::RemoteZygote::Insert(). +// +// Each ld::RemoteZygote::Insert() call consumes a Linker object to distill it +// into the zygote, and yields a ld::RemoteZygote::Domain object that gives the +// entry point address and stack size for that dynamic linking session's main +// (first) root module (usually the main executable), along with the passive +// ABI root symbol addresses for that session's namespace. +// +// Multiple remote dynamic linking sessions meant to share an address space can +// be merged into a single zygote with multiple Insert() calls. Separate +// zygote objects can also be combined using the += or + operators. This +// simplifes repeated loading of an identical whole constellation of dynamic +// linking namespaces sharing a process address space. +// +// Separate ld::RemoteZygote objects can also be used to load multiple sessions +// into the same process. For example, a driver host zygote can be kept +// separate from particular driver zygotes (for individual drivers, or merged +// zygotes for multiple related drivers); thus the same driver host zygote +// could be loaded alongside disparate driver zygotes in different processes. + +// ld::RemoteZygoteVmo is the type of the template parameter to +// ld::RemoteZygote that says how to "own" original file VMO handles. +// +// The ld::RemoteDynamicLinker object owns segment VMOs for relocated (and +// partial-page zeroed) segments, which are transferred into the zygote by +// Insert(). But it doesn't directly own the VMO handles for each module's +// whole original ELF file (read-only, executable handles to VMOs ultimately +// owned by the filesystem). Those are owned indirectly by reference-counted +// ld::RemoteDecodedModule objects the ld::RemoteDynamicLinker::modules() list +// points to. +// +// The zygote needs access to these VMO handles, but it doesn't need to own +// them. On the other hand, it doesn't need any other information from the +// ld::RemoteDecodedModule object. So there are two options for what the +// zygote will own: +// +// * a zx::vmo for the VMO handle +// +// * a Linker::Module::Decoded::Ptr for the module that owns the VMO +// +// The size of the ld::RemoteZygote object is about the same either way, but a +// Decoded::Ptr references a second object (which itself also owns a read-only +// mapping of the whole ELF file into the current address space). If there are +// other references to these ld::RemoteDecodedModule objects anyway in a cache +// of decoded modules reused for new dynamic linking sessions, then holding +// another reference in the zygote saves the overhead of duplicating the VMO +// handle. Doing that caching is probably preferable to recreating new +// ld::RemoteDecodedModule objects (and their whole-file mappings) for the same +// underlying file VMO later. It has nontrivial setup overhead that can be +// amortized; while the memory overhead to maintain it is fairly modest when +// the original file VMO itself is being kept alive anyway. +// +// However, if no further dynamic linking sessions will reuse the same file +// VMOs, then that modest memory overhead can be eliminated by dropping all +// references to the ld::RemoteDecodedModule after duplicating its VMO handle. +// +// ld::RemoteZygoteVmo::kDecodedPtr says to store the reference, while +// ld::RemoteZygoteVmo::kVmo says to store just the zx::vmo handle. +// +// A zygote of either type can be spliced into a zygote that uses zx::vmo. +enum class RemoteZygoteVmo : bool { kVmo = false, kDecodedPtr = true }; + +// This is the type returned by ld::RuntimeDynamicLinker::Insert() on success, +// usually used via the ld::RuntimeDynamicLinker::Domain alias. Each time an +// ld::RuntimeDynamicLinker object is inserted into a zygote, Insert() returns +// this object to describe that dynamic linking domain. This provides the +// absolute runtime addresses for the root module's entry point and for the +// root passive ABI data structures; and the stack size request from the root +// module. These are all the details usually needed either to launch a main +// executable, or to assimilate a new linking domain via its passive ABI and/or +// via some custom entry-point protocol for the root module. +template > +class RemoteZygoteDomain { + public: + using ExecInfo = typename RemoteDecodedModule::ExecInfo; + using size_type = typename Elf::size_type; + + // Return the absolute runtime PC of the root module's entry point. + // For the main executable, this is the PC to pass to zx_process_start. + size_type main_entry() const { return exec_info_.relative_entry; } + + // Return the root module (main executable)'s requested stack size, if any. + std::optional main_stack_size() const { return exec_info_.stack_size; } + + // Return the absolute runtime addresses of the passive ABI symbols in the + // loaded stub dynamic linker image. + size_type abi_vaddr() const { return abi_vaddr_; } + size_type rdebug_vaddr() const { return rdebug_vaddr_; } + + private: + template + friend class RemoteZygote; + + ExecInfo exec_info_; + size_type abi_vaddr_ = 0, rdebug_vaddr_ = 0; +}; + +// The zygote is default-constructible and movable, but not copyable. +// Underneath it just holds a std::vector, so it's cheaper to just move it +// around or leave an unused default-constructed one around than to use +// something like std::unique_ptr. +template > +class RemoteZygote { + public: + using Linker = RemoteDynamicLinker; + using Loader = typename Linker::Module::Loader; + using LoadInfo = typename Linker::Module::LoadInfo; + using ExecInfo = typename Linker::Module::ExecInfo; + using size_type = typename Elf::size_type; + + // This is returned by Insert(). + using Domain = RemoteZygoteDomain; + + RemoteZygote() = default; + RemoteZygote(RemoteZygote&&) = default; + RemoteZygote& operator=(RemoteZygote&&) = default; + + // Two zygotes can be spliced together by consuming the other one and moving + // all its modules into *this, which is the same as if all the Insert() + // options done on other had been done on *this instead. + RemoteZygote& operator+=(RemoteZygote other) { + auto first = std::make_move_iterator(other.modules_.begin()); + auto last = std::make_move_iterator(other.modules_.end()); + modules_.insert(modules_.end(), first, last); + return *this; + } + + // std::move(zygote1) + std::move(zygote2) + ... + RemoteZygote operator+(RemoteZygote other) && { + *this += std::move(other); + return std::move(*this); + } + + // A default-constructed zygote is empty and does not take much space. + bool empty() const { return modules_.empty(); } + + // This does the same as `*this = {};`. + void clear() { modules_.clear(); } + + // This consumes a Linker, distilling its dynamic linking session into the + // zygote and a Domain object. The Linker should already have been used to + // Load and Commit in the initial process whose VMARs were used to do the + // address layout for the zygote. (The state of that process no longer + // matters. No handles to its VMARs are held after Commit.) Processes + // loaded by the new zygote will start out identical to that process and + // share both its VMOs for read-only segments (including RELRO) and its + // parent VMOs (and their unmodified pages) for relocated data. + // + // This can be used multiple times to add additional dynamic linking domains + // from separate dynamic linking sessions. That modifies the zygote so that + // a process loaded from it loads both sessions' modules, including each + // session's own stub dynamic linker providing its own passive ABI view. + // Inserting multiple sessions into one zygote and then calling Load() is no + // different from creating separate zygote objects for each session and + // calling Load() on all of them. + // + // The only error return is for a failure to duplicate a VMO handle. If + // RemoteZygoteVmo::kDecodedPtr is used, then no error is possible. The + // returned Domain object gives the entry point and stack size request for + // the main module, which is the first root module in the dynamic linker + // session, such as the main executable; and the passive ABI root addresses. + zx::result Insert(Linker linker) { + // Extract the main-module and passive ABI bits from the linker to return. + zx::result result = zx::ok(Domain{}); + result->exec_info_ = linker.main_module().decoded().exec_info(); + result->exec_info_.relative_entry += linker.main_module().load_bias(); + result->abi_vaddr_ = linker.abi_vaddr(); + result->rdebug_vaddr_ = linker.rdebug_vaddr(); + + // Reduce each Linker::Module to a ZygoteModule. + modules_.reserve(linker.modules().size()); + for (auto& module : linker.modules()) { + if (module.preloaded()) { + // A module that was preloaded in the linker is treated as preloaded by + // the zygote too: there is nothing to do to load it. This does the + // right thing when distilling a zygote from a secondary dynamic + // linking session: the modules from the first dynamic linking session + // should already be in the zygote; or, equivalently, in a separate + // zygote that will be loaded along with this one. + continue; + } + + // All we need from the decoded module now is its VMO handle for the + // original file. Either duplicate that handle now, or just hold a + // reference to the DecodedModule that owns it. + auto vmo = HoldVmo(module.decoded_module()); + if (vmo.is_error()) { + return vmo.take_error(); + } + modules_.emplace_back(std::move(module), *std::move(vmo)); + } + + return result; + } + + // If this is a RemoteZygoteVmo::kVmo object, then another zygote object of + // the RemoteZygoteVmo::kDecodedPtr flavor can also be spliced in. This + // always consumes that object and will drop all its ld::RemoteDecodedModule + // references. It can fail when duplicating the VMO handles owned by them. + zx::result<> Splice(RemoteZygote other) { + if constexpr (Vmo == RemoteZygoteVmo::kDecodedPtr) { + // When the types match, splicing cannot fail so += supports it. + *this += std::move(other); + } else { + // Convert DecodedPtr modules to zx::vmo modules. + modules_.reserve(modules_.size() + other.modules_.size()); + for (auto& module : other.modules_) { + auto vmo = HoldVmo(module.vmo_holder()); + if (vmo.is_error()) { + return vmo.take_error(); + } + modules_.emplace_back(std::move(module), *std::move(vmo)); + } + } + return zx::ok(); + } + + // Clone the prototype process into another new process. The VMAR must be + // some process's root VMAR or a sub-VMAR covering the same address space + // bounds as the prototype process VMAR passed to Linker::Allocate. + // + // This is non-destructive to the RemoteZygote object, and no references to + // this object are made so its lifetime has no effect on the target process. + // There is no separate Commit phase. When this returns false after an error + // in loading, any sub-VMARs already created will have been destroyed. + // + // On success, a closed VMAR for each module has all its mappings in place; + // they cannot be modified. While zx_vmar_destroy cannot be used since there + // is no handle to them, it's always possible to destroy a whole VMAR as an + // atomic unit with a zx_vmar_unmap call on a handle to a containing VMAR. + // + // It's not advised to use a Diagnostics object that says to keep going for + // errors in this call. If the Diagnostics object does say to keep going, + // this call will fail and return early for some errors but for others it + // will keep going as directed and if it returned true after errors it may + // have left partial mappings in the process. + template + bool Load(Diagnostics& diag, zx::unowned_vmar vmar) const { + // Find the base of the VMAR to calculate offsets from absolute addresses. + zx_vaddr_t vmar_base = 0; + if (auto result = GetVmarBase(*vmar); result.is_ok()) { + vmar_base = *result; + } else { + return diag.SystemError("cannot get ZX_INFO_VMAR: ", + elfldltl::ZirconError{result.error_value()}); + } + + // Collect all the loaders so none gets committed before all are loaded. + std::vector loaders; + loaders.reserve(modules_.size()); + for (const ZygoteModule& module : modules_) { + if (!module.Load(diag, loaders.emplace_back(*vmar), vmar_base)) { + return false; + } + } + + // Now that all modules have been loaded, commit them all. + // Any return before this point destroys all the new VMARs. + for (Loader& loader : loaders) { + // There is no RELRO protection change to be done in the remote case, so + // the Relro object returned can be safely destroyed to close the only + // handle to each module's VMAR and prevent future mapping changes. + constexpr typename LoadInfo::Region kNoRelro{}; + std::ignore = std::move(loader).Commit(kNoRelro); + } + + return true; + } + + private: + friend RemoteZygote; + + using LinkerModule = typename Linker::Module; + using DecodedPtr = typename LinkerModule::Decoded::Ptr; + + // Each module holds a reference to a zx::vmo somehow. Either it holds a + // zx::vmo directly or it holds a DecodedPtr whose ->vmo() can be used. + using VmoHolder = std::conditional_t; + + class ZygoteModule { + public: + ZygoteModule() = default; + + ZygoteModule(ZygoteModule&&) = default; + + // This consumes the module by stealing its segment VMOs. + ZygoteModule(LinkerModule&& module, VmoHolder vmo_holder) + : vmo_holder_(std::move(vmo_holder)), + load_bias_{module.load_bias()}, + load_info_{std::move(module.load_info())} {} + + ZygoteModule(typename RemoteZygote::ZygoteModule&& module, + VmoHolder vmo_holder) + : vmo_holder_(std::move(vmo_holder)), + load_bias_{module.load_bias_}, + load_info_{std::move(module.load_info_)} {} + + ZygoteModule& operator=(ZygoteModule&&) = default; + + VmoHolder& vmo_holder() { return vmo_holder_; } + + const zx::vmo& vmo() const { return GetVmo(vmo_holder_); } + + template + bool Load(Diagnostics& diag, Loader& loader, zx_vaddr_t vmar_base) const { + return loader.Allocate(diag, load_info_, VmarOffset(vmar_base)) && + loader.Load(diag, load_info_, vmo().borrow()); + } + + private: + friend RemoteZygote; + + size_t VmarOffset(zx_vaddr_t vmar_base) const { + return Loader::VmarOffsetForLoadBias(vmar_base, load_info_, load_bias_); + } + + VmoHolder vmo_holder_; + size_type load_bias_; + LoadInfo load_info_; + }; + + static const zx::vmo& GetVmo(const DecodedPtr& module) { return module->vmo(); } + + static const zx::vmo& GetVmo(const zx::vmo& vmo) { return vmo; } + + static zx::result HoldVmo(DecodedPtr module) { + if constexpr (Vmo == RemoteZygoteVmo::kVmo) { + // Duplicate the VMO handle owned by the RemoteDecodedModule. + zx::vmo vmo; + zx_status_t status = module->vmo().duplicate(ZX_RIGHT_SAME_RIGHTS, &vmo); + if (status != ZX_OK) { + return zx::error_result{status}; + } + return zx::ok(std::move(vmo)); + } else { + // Holding the original decoded module pointer can never fail. + return zx::ok(std::move(module)); + } + } + + static zx::result GetVmarBase(const zx::vmar& vmar) { + zx_info_vmar_t vmar_info; + zx_status_t status = + vmar.get_info(ZX_INFO_VMAR, &vmar_info, sizeof(vmar_info), nullptr, nullptr); + if (status != ZX_OK) [[unlikely]] { + return zx::error{status}; + } + return zx::ok(vmar_info.base); + } + + std::vector modules_; +}; + +} // namespace ld + +#endif // LIB_LD_REMOTE_ZYGOTE_H_ diff --git a/sdk/lib/ld/test/BUILD.gn b/sdk/lib/ld/test/BUILD.gn index 32c42e69e926..adccaf05993b 100644 --- a/sdk/lib/ld/test/BUILD.gn +++ b/sdk/lib/ld/test/BUILD.gn @@ -37,10 +37,7 @@ test("ld-unittests") { deps += [ ":load-tests" ] } if (is_fuchsia) { - sources += [ - "mock-loader-service-tests.cc", - "remote-tests.cc", - ] + sources += [ "mock-loader-service-tests.cc" ] } } @@ -118,6 +115,7 @@ source_set("load-tests") { "ld-startup-in-process-tests-zircon.h", "ld-startup-spawn-process-tests-zircon.cc", "ld-startup-spawn-process-tests-zircon.h", + "remote-tests.cc", ] deps += [ "..:ld-stub.basic", diff --git a/sdk/lib/ld/test/ld-load-zircon-process-tests-base.cc b/sdk/lib/ld/test/ld-load-zircon-process-tests-base.cc index faf8deed7e6f..76d2898a3401 100644 --- a/sdk/lib/ld/test/ld-load-zircon-process-tests-base.cc +++ b/sdk/lib/ld/test/ld-load-zircon-process-tests-base.cc @@ -22,13 +22,13 @@ void LdLoadZirconProcessTestsBase::set_process(zx::process process) { int64_t LdLoadZirconProcessTestsBase::Wait() { int64_t result = -1; - - auto wait_for_termination = [this, &result]() { + auto wait_for_termination = [process = std::exchange(process_, {}), &result]() { + ASSERT_TRUE(process) << "Wait() called before Init()?"; zx_signals_t signals; - ASSERT_EQ(process_.wait_one(ZX_PROCESS_TERMINATED, zx::time::infinite(), &signals), ZX_OK); + ASSERT_EQ(process.wait_one(ZX_PROCESS_TERMINATED, zx::time::infinite(), &signals), ZX_OK); ASSERT_TRUE(signals & ZX_PROCESS_TERMINATED); zx_info_process_t info; - ASSERT_EQ(process_.get_info(ZX_INFO_PROCESS, &info, sizeof(info), nullptr, nullptr), ZX_OK); + ASSERT_EQ(process.get_info(ZX_INFO_PROCESS, &info, sizeof(info), nullptr, nullptr), ZX_OK); ASSERT_TRUE(info.flags & ZX_INFO_PROCESS_FLAG_STARTED); ASSERT_TRUE(info.flags & ZX_INFO_PROCESS_FLAG_EXITED); result = info.return_code; @@ -91,7 +91,7 @@ int64_t LdLoadZirconProcessTestsBase::Run( // TestProcessArgs* bootstrap, std::optional stack_size, const zx::thread& thread, uintptr_t entry, uintptr_t vdso_base, const zx::vmar& root_vmar) { Start(bootstrap, {}, stack_size, thread, entry, vdso_base, root_vmar); - return ::testing::Test::HasFailure() ? -1 : Wait(); + return ::testing::Test::HasFatalFailure() ? -1 : Wait(); } LdLoadZirconProcessTestsBase::~LdLoadZirconProcessTestsBase() { diff --git a/sdk/lib/ld/test/ld-load-zircon-process-tests-base.h b/sdk/lib/ld/test/ld-load-zircon-process-tests-base.h index d72754ff6921..11d878a6c27f 100644 --- a/sdk/lib/ld/test/ld-load-zircon-process-tests-base.h +++ b/sdk/lib/ld/test/ld-load-zircon-process-tests-base.h @@ -46,6 +46,7 @@ class LdLoadZirconProcessTestsBase : public LdLoadZirconLdsvcTestsBase { const zx::vmar& root_vmar); // Wait for the process to die and collect its exit code. + // This clears the process() so a new one can be installed. int64_t Wait(); private: diff --git a/sdk/lib/ld/test/ld-remote-process-tests.h b/sdk/lib/ld/test/ld-remote-process-tests.h index 12e2de61c1ef..014334239096 100644 --- a/sdk/lib/ld/test/ld-remote-process-tests.h +++ b/sdk/lib/ld/test/ld-remote-process-tests.h @@ -102,10 +102,16 @@ class LdRemoteProcessTests : public ::testing::Test, public LdLoadZirconProcessT const zx::vmar& root_vmar() { return root_vmar_; } + uintptr_t entry() const { return entry_; } + void set_entry(uintptr_t entry) { entry_ = entry; } + uintptr_t vdso_base() const { return vdso_base_; } + void set_vdso_base(uintptr_t vdso_base) { vdso_base_ = vdso_base; } + std::optional stack_size() const { return stack_size_; } + void set_stack_size(std::optional stack_size) { stack_size_ = stack_size; } private: diff --git a/sdk/lib/ld/test/modules/BUILD.gn b/sdk/lib/ld/test/modules/BUILD.gn index faae4b97fd21..89f740c71e8d 100644 --- a/sdk/lib/ld/test/modules/BUILD.gn +++ b/sdk/lib/ld/test/modules/BUILD.gn @@ -33,6 +33,8 @@ in_process_executables = [ ":tls-exec-shlib", ":tls-gd", ":tls-desc", + ":zygote", + ":zygote-secondary", ] non_module_in_process_executables = [ @@ -68,6 +70,7 @@ shlibs = [ ":tls-desc-dep", ":has-missing-dep", ":second-session-module-deps-a", + ":zygote-dep", ] modules = [ ":second-session-module" ] @@ -896,3 +899,44 @@ test_shared_library("second-session-module-deps-a") { sources = [ "second-session-module-deps-a.cc" ] deps = [ "//zircon/system/public" ] } + +test_executable("zygote") { + can_be_in_process = true + ldflags = [ + "-Wl,-soname,libzygote-test.so.1", + "-rdynamic", + ] + sources = [ "zygote.cc" ] + shlib_deps = [ ":zygote-dep" ] + deps = [ + "//zircon/system/public", + + # This creates a DT_NEEDED on the executable's own SONAME, which should + # have no effect. It also grants `gn check` access to the zygote.h header. + ":zygote.lib", + ] +} + +ifs_shared_library("zygote.lib") { + testonly = true + public = [ "zygote.h" ] + abi = "zygote.ifs" +} + +test_shared_library("zygote-dep") { + sources = [ "zygote-dep.cc" ] + deps = [ + ":zygote.lib", + "//zircon/system/public", + ] +} + +test_executable("zygote-secondary") { + can_be_in_process = true + sources = [ "zygote-secondary.cc" ] + shlib_deps = [ ":zygote-dep" ] + deps = [ + ":zygote.lib", + "//zircon/system/public", + ] +} diff --git a/sdk/lib/ld/test/modules/zygote-dep.cc b/sdk/lib/ld/test/modules/zygote-dep.cc new file mode 100644 index 000000000000..241a0970f5f7 --- /dev/null +++ b/sdk/lib/ld/test/modules/zygote-dep.cc @@ -0,0 +1,9 @@ +// Copyright 2024 The Fuchsia Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include + +#include "zygote.h" + +__EXPORT int zygote_dep() { return called_by_zygote_dep() + *initialized_data[1]++; } diff --git a/sdk/lib/ld/test/modules/zygote-secondary.cc b/sdk/lib/ld/test/modules/zygote-secondary.cc new file mode 100644 index 000000000000..615f9db5bcb9 --- /dev/null +++ b/sdk/lib/ld/test/modules/zygote-secondary.cc @@ -0,0 +1,19 @@ +// Copyright 2024 The Fuchsia Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include + +#include "test-start.h" +#include "zygote.h" + +// The zygote-dep in the secondary domain will call this function, while the +// zygote-dep in the first domain calls the one in the main zygote executable. +// However, initialized_data here resolves to the executable's symbol, because +// it's also in the secondary domain's resolution list, but after this module. +__EXPORT int called_by_zygote_dep() { + static int** data = &initialized_data[2]; + return *(*data--)++ + zygote_test_main(); +} + +extern "C" int64_t TestStart() { return zygote_dep() + 1; } diff --git a/sdk/lib/ld/test/modules/zygote.cc b/sdk/lib/ld/test/modules/zygote.cc new file mode 100644 index 000000000000..8db1cc5e7a13 --- /dev/null +++ b/sdk/lib/ld/test/modules/zygote.cc @@ -0,0 +1,26 @@ +// Copyright 2024 The Fuchsia Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "zygote.h" + +#include + +#include "test-start.h" + +// This is initialized data. If this is shared across two calls to +// called_by_zygote_dep(), the second call will see different values. +static int counters[] = {6, 7, 8}; +__EXPORT int* initialized_data[] = {&counters[2], &counters[1], &counters[0]}; + +// This is called by zygote_dep(). If its bss is shared across two calls, the +// second call will see a different value. +__EXPORT int called_by_zygote_dep() { + static int bss; + return *initialized_data[++bss]++; +} + +// This is called by zygote-secondary. +__EXPORT int zygote_test_main() { return zygote_dep() + 1; } + +extern "C" int64_t TestStart() { return zygote_test_main() + 1; } diff --git a/sdk/lib/ld/test/modules/zygote.h b/sdk/lib/ld/test/modules/zygote.h new file mode 100644 index 000000000000..37b0b174110c --- /dev/null +++ b/sdk/lib/ld/test/modules/zygote.h @@ -0,0 +1,19 @@ +// Copyright 2024 The Fuchsia Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef LIB_LD_TEST_MODULES_ZYGOTE_H_ +#define LIB_LD_TEST_MODULES_ZYGOTE_H_ + +// This is defined in the zygote-dep shared library. +extern "C" int zygote_dep(); + +// It uses both of these, which are defined in the zygote main executable. +extern "C" int called_by_zygote_dep(); +extern int* initialized_data[]; + +// This is defined by the zygote main executable, but only referenced by the +// zygote-secondary module. +extern "C" int zygote_test_main(); + +#endif // LIB_LD_TEST_MODULES_ZYGOTE_H_ diff --git a/sdk/lib/ld/test/modules/zygote.ifs b/sdk/lib/ld/test/modules/zygote.ifs new file mode 100644 index 000000000000..25676f4f26c9 --- /dev/null +++ b/sdk/lib/ld/test/modules/zygote.ifs @@ -0,0 +1,8 @@ +--- !ifs-v1 +IfsVersion: 3.0 +SoName: libzygote-test.so.1 +Symbols: + - { Name: called_by_zygote_dep, Type: Func } + - { Name: initialized_data, Type: Object, Size: 8 } + - { Name: zygote_test_main, Type: Func } +... diff --git a/sdk/lib/ld/test/remote-tests.cc b/sdk/lib/ld/test/remote-tests.cc index 3c4f83bc51a9..b88d45786007 100644 --- a/sdk/lib/ld/test/remote-tests.cc +++ b/sdk/lib/ld/test/remote-tests.cc @@ -5,6 +5,7 @@ #include #include #include +#include #include @@ -399,6 +400,168 @@ TEST_F(LdRemoteTests, SecondSession) { ExpectLog(""); } +TEST_F(LdRemoteTests, Zygote) { + constexpr int64_t kReturnValue = 17; + constexpr int64_t kSecondaryReturnValue = 23; + constexpr int kZygoteCount = 10; + + auto diag = elfldltl::testing::ExpectOkDiagnostics(); + + const ld::RemoteAbiStub<>::Ptr abi_stub = + ld::RemoteAbiStub<>::Create(diag, TakeStubLdVmo(), kPageSize); + ASSERT_TRUE(abi_stub); + + // Linker::Module::Decoded and ZygoteLinker::Module::Decoded are the same but + // Linker and ZygoteLinker are not quite the same. + using ZygoteLinker = ld::RemoteZygote<>::Linker; + ZygoteLinker linker{abi_stub}; + + zx::vmo exec_vmo = GetExecutableVmo("zygote"); + EXPECT_TRUE(exec_vmo); + zx::vmo vdso_vmo; + zx_status_t status = ld::testing::GetVdsoVmo()->duplicate(ZX_RIGHT_SAME_RIGHTS, &vdso_vmo); + EXPECT_EQ(status, ZX_OK) << zx_status_get_string(status); + + Linker::Module::DecodedPtr executable = + Linker::Module::Decoded::Create(diag, std::move(exec_vmo), kPageSize); + Linker::Module::DecodedPtr vdso = + Linker::Module::Decoded::Create(diag, std::move(vdso_vmo), kPageSize); + ASSERT_TRUE(executable); + ASSERT_TRUE(vdso); + + ZygoteLinker::InitModuleList init_modules{{ + ZygoteLinker::Executable(std::move(executable)), + ZygoteLinker::Implicit(std::move(vdso)), + }}; + auto get_dep = GetDepFunction(diag); // Needed primes this. + ASSERT_NO_FATAL_FAILURE(Needed({"libzygote-dep.so"})); + auto init_result = linker.Init(diag, std::move(init_modules), get_dep); + ASSERT_TRUE(init_result); + VerifyAndClearNeeded(); + + // Create a process that will be the first to run. Its ASLR will choose the + // load addresses used again for all later zygote processes. + ASSERT_NO_FATAL_FAILURE(Init()); + + EXPECT_TRUE(linker.Allocate(diag, root_vmar().borrow())); + ASSERT_TRUE(linker.Relocate(diag)); + ASSERT_TRUE(linker.Load(diag)); + + // Collect what's needed to start the process. + const ZygoteLinker::Module& loaded_vdso = *init_result->back(); + set_vdso_base(loaded_vdso.module().vaddr_start()); + set_entry(linker.main_entry()); + set_stack_size(linker.main_stack_size()); + + // The prototype process is ready to start. The linker object is now + // consumed in making the zygote. + linker.Commit(); + + // Capture the settled load details of the executable and vDSO for later. + ZygoteLinker::InitModuleList secondary_init_modules = linker.PreloadedImplicit(*init_result); + + // Make a zygote that holds onto the DecodedPtr references. + ld::RemoteZygote original_zygote; + auto result = original_zygote.Insert(std::move(linker)); + ASSERT_TRUE(result.is_ok()) << result.status_string(); + EXPECT_EQ(result->main_entry(), entry()); + EXPECT_EQ(result->main_stack_size(), stack_size()); + + // The += operator allows for splicing that cannot fail, since the DecodedPtr + // references just transfer from the other zygote. + original_zygote += ld::RemoteZygote{}; + + // Run the prototype process to completion. It will have changed its segment + // contents, but they should not be shared with later runs. + EXPECT_EQ(Run(), kReturnValue); + ExpectLog(""); + + // Move into a zygote that owns only zx::vmo and not DecodedPtr. Splicing + // into this from the zygote that owns DecodedPtr instead can fail. + ld::RemoteZygote zygote; + auto splice = zygote.Splice(std::move(original_zygote)); + EXPECT_TRUE(splice.is_ok()) << splice.status_string(); + + // The += operator allows for splicing that cannot fail, since the other + // object already owns zx::vmo handles directly and they just transfer. + zygote += ld::RemoteZygote{}; + + for (int i = 1; i <= kZygoteCount; ++i) { + // Make a new process. + ASSERT_NO_FATAL_FAILURE(Init()) << "zygote child " << i << " of " << kZygoteCount; + + // Load it up from the zygote. + EXPECT_TRUE(zygote.Load(diag, root_vmar().borrow())) + << "zygote child " << i << " of " << kZygoteCount; + + // Run it to completion. It would go wrong or return the wrong value if + // its segments had been written by an earlier run. + EXPECT_EQ(Run(), kReturnValue) << "zygote child " << i << " of " << kZygoteCount; + ExpectLog(""); + } + + // Start a new process for the secondary session test. + ASSERT_NO_FATAL_FAILURE(Init()) << "secondary"; + + // First the new process gets loaded up from the zygote like the others. + EXPECT_TRUE(zygote.Load(diag, root_vmar().borrow())) << "secondary"; + + // Fetch the secondary domain's root module. It's built and packaged as an + // executable since it has an entry point that acts like one. + constexpr ZygoteLinker::Soname kSecondaryName{"zygote-secondary"}; + zx::vmo secondary_vmo; + ASSERT_NO_FATAL_FAILURE(secondary_vmo = GetExecutableVmo(kSecondaryName.str())); + EXPECT_TRUE(secondary_vmo); + Linker::Module::DecodedPtr secondary = + Linker::Module::Decoded::Create(diag, std::move(secondary_vmo), kPageSize); + + // Now start the secondary session. The secondary_init_modules list + // collected above still corresponds to where the zygote loaded things. + ZygoteLinker secondary_linker{abi_stub}; + secondary_init_modules.emplace_back( + ZygoteLinker::RootModule(std::move(secondary), kSecondaryName)); + ASSERT_NO_FATAL_FAILURE(Needed({"libzygote-dep.so"})); + auto secondary_init_result = + secondary_linker.Init(diag, std::move(secondary_init_modules), get_dep); + ASSERT_TRUE(secondary_init_result); + + EXPECT_TRUE(secondary_linker.Allocate(diag, root_vmar().borrow())); + ASSERT_TRUE(secondary_linker.Relocate(diag)); + ASSERT_TRUE(secondary_linker.Load(diag)); + + // This process will start at the secondary module's entry point rather than + // the original executable's. The set_vdso_base() call above is still in + // force, since that has not changed since the original session. + set_entry(secondary_linker.main_entry()); + set_stack_size(secondary_linker.main_stack_size()); + + // The prototype secondary process is ready to start. + secondary_linker.Commit(); + + // Consume the secondary_linker object in the existing zygote, so now it will + // load both the original and secondary modules into each new process. + auto secondary_result = zygote.Insert(std::move(secondary_linker)); + ASSERT_TRUE(secondary_result.is_ok()) << secondary_result.status_string(); + EXPECT_EQ(secondary_result->main_entry(), entry()); + EXPECT_EQ(secondary_result->main_stack_size(), stack_size()); + + // Run the prototype secondary process to completion. + EXPECT_EQ(Run(), kSecondaryReturnValue); + ExpectLog(""); + + // Test the combined zygote behaves like the secondary prototype over again. + for (int i = 1; i <= kZygoteCount; ++i) { + ASSERT_NO_FATAL_FAILURE(Init()) << "secondary zygote child " << i << " of " << kZygoteCount; + + EXPECT_TRUE(zygote.Load(diag, root_vmar().borrow())) + << "secondary zygote child " << i << " of " << kZygoteCount; + + EXPECT_EQ(Run(), kSecondaryReturnValue) + << "secondary zygote child " << i << " of " << kZygoteCount; + ExpectLog(""); + } +} + TEST_F(LdRemoteTests, RemoteAbiStub) { auto diag = elfldltl::testing::ExpectOkDiagnostics(); diff --git a/src/lib/elfldltl/include/lib/elfldltl/segment-with-vmo.h b/src/lib/elfldltl/include/lib/elfldltl/segment-with-vmo.h index 21f125ebacdc..e62d03755577 100644 --- a/src/lib/elfldltl/include/lib/elfldltl/segment-with-vmo.h +++ b/src/lib/elfldltl/include/lib/elfldltl/segment-with-vmo.h @@ -6,6 +6,7 @@ #define SRC_LIB_ELFLDLTL_INCLUDE_LIB_ELFLDLTL_SEGMENT_WITH_VMO_H_ #include +#include #include #include @@ -72,6 +73,16 @@ class SegmentWithVmo { zx::vmo& vmo() { return vmo_; } const zx::vmo& vmo() const { return vmo_; } + // Replace the VMO handle with an immutable one so no further modifications + // can be made. Note that writable mappings may already exist, but no new + // writable mappings can be made with vmo() after this. + zx::result<> MakeImmutable() { + if (vmo_) { + return zx::make_result(vmo_.replace(kReadonlyRights, &vmo_)); + } + return zx::ok(); + } + private: zx::vmo vmo_; }; @@ -147,6 +158,7 @@ class SegmentWithVmo { if (*other_vmo) { assert(copy.filesz() == other.filesz()); + zx_status_t status = // CopyVmo(other_vmo->borrow(), 0, copy.filesz(), copy.vmo()); if (status != ZX_OK) [[unlikely]] { @@ -218,6 +230,31 @@ class SegmentWithVmo { using NoCopy = Wrapper::template Type; using NoCopySegmentVmo = Wrapper::SegmentVmo; + // What's not obvious is that ZX_DEFAULT_VMO_RIGHTS &~ ZX_VM_RIGHT_WRITE + // cannot be used here. The handle from the VMO created in MakeMutable will + // certainly have ZX_VM_RIGHT_WRITE, but it won't necessarily have all the + // rights in ZX_DEFAULT_VMO_RIGHTS. The zx_handle_replace system call does + // not have a way to mask off rights, only choose a whole new set of rights + // that must be a subset of the existing rights--so you have to be sure + // what's truly a subset of the existing rights. + // + // We shouldn't presume exactly what rights the filesystem or loader service + // or whatnot gave us on the file VMO handle. The per-segment VMOs are from + // zx_vmo_create_child (in CopyVmo, below), which returns the new VMO via a + // handle with rights that depend on the original VMO handle's rights. So + // even though segment.vmo() is a handle we made ourselves, we can't presume + // exactly what rights it has. + // + // So to drop ZX_RIGHT_WRITE for the readonly case below, we'd need to do one + // of two things: do a ZX_INFO_HANDLE_BASIC query to find the rights before + // masking off ZX_RIGHT_WRITE in zx_handle_replace; or, just use a fixed set + // of rights that's small enough to be surely a subset of the VMO rights we + // have (as derived from the file VMO handle's rights by the behavior of + // zx_vmo_create_child). Since we know how these particular VMOs will need + // to be used later, we do the latter. + static constexpr zx_rights_t kReadonlyRights = + ZX_RIGHTS_BASIC | ZX_RIGHTS_PROPERTY | ZX_RIGHT_MAP | ZX_RIGHT_READ; + // This takes a LoadInfo::*Segment type that has file contents (i.e. not // ZeroFillSegment), and ensures that segment.vmo() is a valid segment. // Unless there's a VMO handle there already, it creates a new copy-on-write @@ -251,35 +288,6 @@ class SegmentWithVmo { using DataWithZeroFillSegment = typename LoadInfo::DataWithZeroFillSegment; auto align_segment = [vmo, page_size, readonly, &diag](auto& segment) { using Segment = std::decay_t; - - // When there's a partial page to zero, this will create a new VMO. With - // the readonly flag, we want to ensure this VMO won't be modified in the - // future, so we want to drop ZX_VM_RIGHT_WRITE from our handle to it. - // What's not obvious is that ZX_DEFAULT_VMO_RIGHTS &~ ZX_VM_RIGHT_WRITE - // cannot be used here. The handle from the VMO created in MakeMutable - // will certainly have ZX_VM_RIGHT_WRITE, but it won't necessarily have - // all the rights in ZX_DEFAULT_VMO_RIGHTS. The zx_handle_replace system - // call does not have a way to mask off rights, only choose a whole new - // set of rights that must be a subset of the existing rights--so you - // have to be sure what's truly a subset of the existing rights. - // - // We shouldn't presume exactly what rights the filesystem or loader - // service or whatnot gave us on the file VMO handle. The per-segment - // VMOs are from zx_vmo_create_child (in CopyVmo, below), which returns - // the new VMO via a handle with rights that depend on the original VMO - // handle's rights. So even though segment.vmo() is a handle we made - // ourselves, we can't presume exactly what rights it has. - // - // So to drop ZX_RIGHT_WRITE for the readonly case below, we'd need to do - // one of two things: do a ZX_INFO_HANDLE_BASIC query to find the rights - // before masking off ZX_RIGHT_WRITE in zx_handle_replace; or, just use a - // fixed set of rights that's small enough to be surely a subset of the - // VMO rights we have (as derived from the file VMO handle's rights by - // the behavior of zx_vmo_create_child). Since we know how these - // particular VMOs will need to be used later, we do the latter. - constexpr zx_rights_t kReadonlyRights = - ZX_RIGHTS_BASIC | ZX_RIGHTS_PROPERTY | ZX_RIGHT_MAP | ZX_RIGHT_READ; - if constexpr (std::is_same_v) { const size_t zero_size = segment.MakeAligned(page_size); if (zero_size > 0) { @@ -287,15 +295,21 @@ class SegmentWithVmo { if (!MakeMutable(diag, segment, vmo->borrow())) [[unlikely]] { return false; } + const uint64_t zero_offset = segment.filesz() - zero_size; zx_status_t status = ZeroVmo(segment.vmo().borrow(), zero_offset, zero_size); if (status != ZX_OK) [[unlikely]] { return SystemError(diag, segment, kZeroVmoFail, status); } + + // When there's a partial page to zero, this will create a new VMO. + // With the readonly flag, we want to ensure this VMO won't be + // modified in the future, so we want to drop ZX_VM_RIGHT_WRITE + // from our handle to it. if (readonly) { - status = segment.vmo().replace(kReadonlyRights, &segment.vmo()); - if (status != ZX_OK) [[unlikely]] { - return SystemError(diag, segment, kProtectVmoFail, status); + zx::result<> result = segment.MakeImmutable(); + if (result.is_error()) [[unlikely]] { + return SystemError(diag, segment, kProtectVmoFail, result.error_value()); } } }