From 3ba8fce249c6ae6ab70f768decfad6312a209313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20I=C3=B1igo=20Blasco?= Date: Mon, 3 Aug 2026 09:46:24 +0200 Subject: [PATCH 1/2] feat(pj_base,pj_plugins): read plugin manifests without loading the DSO MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary - **New descriptor section.** Every DSO built with a `PJ_*_PLUGIN` macro now emits a self-describing blob — magic, ABI version, family, manifest JSON — into a dedicated section: `.pj_manifest` (ELF), `.pjmani` (PE), `__PJ,__manifest` (Mach-O). It is located by section, not by symbol, so it survives stripping. See `pj_base/include/pj_base/plugin_descriptor_section.hpp`. - **New object-file reader** (`pj_plugins/src/detail/descriptor_section_reader.{hpp,cpp}`) for ELF32/64 (both endiannesses), PE and thin Mach-O. Fields are read at fixed offsets rather than through `` / `` / ``, so all three parsers compile and are testable on any host. Only the container headers and the section itself are read: inspecting a 50 MB plugin costs a few KB and ~2 ms. - **`inspectPluginDso` prefers the static path** and only falls back to `dlopen` + vtable probe + `dlclose` for a DSO with no descriptor section. - **An ABI mismatch is now rejected from the section alone**, so a plugin built against a different ABI is no longer loaded in order to discover that it should not have been. ## Why Discovery obtained the manifest by `dlopen`'ing each candidate, calling its vtable entry point, and `dlclose`'ing. That leaves the plugin resident. glibc pins a library `NODELETE` as soon as it defines an `STB_GNU_UNIQUE` symbol, which any vague-linkage static produces (inline-function locals, template statics, Meyers singletons). When a second copy of the same plugin is later loaded from a different path, it binds its own references to the first copy's storage and finds the initialisation guards already set — so its own constructors never run and any layout drift between the two builds corrupts the process. This is not theoretical: all 22 bundled plugins export `UNIQUE` symbols (one representative plugin exports 1117 of them, another 358). Even first-party plugins built with `pj_emit_plugin_manifest` and its `-fvisibility=hidden` export some, because libstdc++ declares `namespace std` with explicit default visibility, so `__gen_vtable` / `__to_chars_10_impl` instantiations reach `.dynsym` anyway. Two reproducible startup crashes, both fixed here: | Scenario | Crash site before | |---|---| | A locally built copy loaded via `--plugin-dir` over a bundled plugin | `PluginRuntimeCatalog::collectDeduplicatedPlugins` | | A marketplace copy newer than the bundled one — no CLI flag involved | `ExtensionCatalogService::seedBundledPlugins` | ## Notes - **Vtable-shape validation moves out of discovery** for plugins taking the static path (`protocol_version`, `struct_size`, required slots). The contract is not loosened: the family loaders (`data_source_library.cpp` and siblings) already validate every plugin at load time. The two deliberately-broken test plugins hand-write their vtables, so they have no descriptor section, take the fallback, and their diagnostics are unchanged. - **Plugins must be rebuilt to benefit.** A DSO built against an older SDK has no section and still takes the `dlopen` path — including prebuilt third-party plugins that stay exposed until they are rebuilt against this SDK. - **`PJ_DIALOG_PLUGIN`'s body moved into `PJ_DIALOG_PLUGIN_IMPL`.** Only the form taking a manifest emits a blob; the manifest-less form still routes to the static-link getter under `PJ_STATIC_PLUGINS`. - Each plugin gains one exported symbol per family (`pj_plugin_descriptor_*`). With `-DPJ_ENABLE_ABI_CHECK=ON`, `abidiff` reports it as an addition (libabigail bit 4, warning), not an incompatible change. `abi/baseline.abi` is untouched. ## Testing - `./build.sh --debug && ./test.sh` — full suite under ASAN. - `ctest -R descriptor_section_reader_test` — 18 cases over synthetic ELF64/ELF32/big-endian ELF/PE/Mach-O containers, including several blobs separated by linker padding, a section name sharing the prefix, PE file-alignment padding, an unknown blob version, and a manifest length running past the section. - `ctest -R plugin_catalog_test` — in particular `InspectingADsoDoesNotLeaveItMapped`, which fails if anyone reintroduces a `dlopen` into discovery, and `MultiFamilyDsoReportsTheHighestPrecedenceFamily`. - End-to-end against a real host build: place two divergent builds of one plugin id in the bundled dir and in a `--plugin-dir` override, then repeat with the second copy in the marketplace dir instead. Both used to SIGSEGV at startup. Expect no crash, and exactly one plugin load — the winning copy — instead of one per path. - Confirm the fallback still discovers plugins built against the previous SDK (they report zero `.pj_manifest` sections under `readelf -S`). - Worth checking on Windows: the PE path is covered by unit tests but has not run against a real `.dll`. --- pj_base/CLAUDE.md | 2 +- .../pj_base/plugin_descriptor_section.hpp | 184 +++++++ .../pj_base/sdk/data_source_plugin_base.hpp | 4 +- .../pj_base/sdk/toolbox_plugin_base.hpp | 4 +- pj_plugins/CLAUDE.md | 4 +- pj_plugins/CMakeLists.txt | 14 +- .../pj_plugins/sdk/dialog_plugin_base.hpp | 15 +- pj_plugins/docs/ARCHITECTURE.md | 37 +- .../sdk/message_parser_plugin_base.hpp | 4 +- .../src/detail/descriptor_section_reader.cpp | 487 +++++++++++++++++ .../src/detail/descriptor_section_reader.hpp | 43 ++ pj_plugins/src/plugin_catalog.cpp | 64 +++ .../tests/descriptor_section_reader_test.cpp | 497 ++++++++++++++++++ pj_plugins/tests/plugin_catalog_test.cpp | 39 ++ 14 files changed, 1381 insertions(+), 17 deletions(-) create mode 100644 pj_base/include/pj_base/plugin_descriptor_section.hpp create mode 100644 pj_plugins/src/detail/descriptor_section_reader.cpp create mode 100644 pj_plugins/src/detail/descriptor_section_reader.hpp create mode 100644 pj_plugins/tests/descriptor_section_reader_test.cpp diff --git a/pj_base/CLAUDE.md b/pj_base/CLAUDE.md index 7ab1133b..b7d5c21f 100644 --- a/pj_base/CLAUDE.md +++ b/pj_base/CLAUDE.md @@ -6,7 +6,7 @@ pj_base is the **Level 0** foundation and the **SDK boundary** for plugin author - `include/pj_base/` — vocabulary primitives: `types.hpp`, `time.hpp` (absolute time spine: `Timepoint`/`Duration` + `fromRaw`/`toRaw`), `type_tree.hpp`, `dataset.hpp`, `expected.hpp`, `span.hpp`, `number_parse.hpp`, `assert.hpp`, `diagnostic_sink.hpp`, `buffer_anchor.hpp`. - `include/pj_base/builtin/` — the 16 builtin object struct headers (`*.hpp`; 17 enum values in `BuiltinObjectType`, values 2 and 12 reserved) + their 15 wire codecs (`*_codec.hpp`; RobotDescription has none) + the `BuiltinObject` (`std::any`) type-erased holder. - `include/pj_base/sdk/` — C++ SDK over the ABI: DataSource + Toolbox `*_plugin_base.hpp`, `service_registry.hpp`/`service_traits.hpp`, host views, Arrow RAII holders, `testing/`. -- `include/pj_base/*_protocol.h`, `plugin_data_api.h`, `builtin_object_abi.h`, `plugin_abi_export.hpp` — the stable C-ABI surface for DataSource/MessageParser/Toolbox (the Dialog protocol header lives in `pj_plugins/dialog_protocol/`). +- `include/pj_base/*_protocol.h`, `plugin_data_api.h`, `builtin_object_abi.h`, `plugin_abi_export.hpp`, `plugin_descriptor_section.hpp` (the statically discoverable manifest blob every `PJ_*_PLUGIN` macro emits) — the stable C-ABI surface for DataSource/MessageParser/Toolbox (the Dialog protocol header lives in `pj_plugins/dialog_protocol/`). - `proto/pj/` — canonical `.proto` wire contracts for the builtin types (see its README). - `src/`, `tests/` — codec/parse impls and gtests. - `abi/baseline.abi` — golden libabigail dump; the ABI-stability regression baseline. diff --git a/pj_base/include/pj_base/plugin_descriptor_section.hpp b/pj_base/include/pj_base/plugin_descriptor_section.hpp new file mode 100644 index 00000000..f94b670a --- /dev/null +++ b/pj_base/include/pj_base/plugin_descriptor_section.hpp @@ -0,0 +1,184 @@ +#ifndef PJ_PLUGIN_DESCRIPTOR_SECTION_HPP +#define PJ_PLUGIN_DESCRIPTOR_SECTION_HPP + +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +#include +#include + +#include "pj_base/plugin_data_api.h" + +// Statically discoverable plugin descriptor. +// +// The host reads a plugin's family and manifest straight out of the DSO image +// on disk, so discovery never has to dlopen the plugin. That matters because +// dlclose does not necessarily unmap what dlopen mapped: glibc pins a DSO +// NODELETE as soon as it defines an STB_GNU_UNIQUE symbol (any vague-linkage +// static — inline-function locals, template statics, Meyers singletons — that +// reaches .dynsym). An inspect-then-close pass therefore leaves the plugin +// resident for the life of the process. If a second copy of the same plugin is +// later loaded from a different path, that copy binds its own references to the +// first copy's storage and finds its initialisation guards already set, so any +// layout drift between the two builds corrupts the process. +// +// Every DSO built with a PJ_*_PLUGIN macro carries one of these blobs per +// family it implements. The blob duplicates the manifest string that is also +// reachable through the vtable; the vtable copy stays authoritative once the +// plugin is genuinely loaded, and vtable-shape validation still happens there. +// +// The blob is located by SECTION, not by symbol name, so it survives stripping +// and needs no dynamic-symbol lookup. + +// --- Section names ----------------------------------------------------------- +// +// All three are defined on every platform: a plugin only ever emits into its +// own container's section, but the host-side reader is compiled everywhere so +// the object-file parsers can be tested on any machine. +// +// PE section names are capped at 8 bytes in the image section header, hence the +// abbreviated name on Windows. Mach-O needs the segment,section pair form. +#define PJ_PLUGIN_DESCRIPTOR_SECTION_NAME_ELF ".pj_manifest" +#define PJ_PLUGIN_DESCRIPTOR_SECTION_NAME_PE ".pjmani" +#define PJ_PLUGIN_DESCRIPTOR_SECTION_NAME_MACHO "__PJ,__manifest" + +// --- Section placement ------------------------------------------------------- +#if defined(_MSC_VER) +#define PJ_PLUGIN_DESCRIPTOR_SECTION_NAME PJ_PLUGIN_DESCRIPTOR_SECTION_NAME_PE +#pragma section(".pjmani", read) +#define PJ_PLUGIN_DESCRIPTOR_PLACEMENT __declspec(allocate(".pjmani")) +// dllexport is what keeps the object alive through /OPT:REF: an unreferenced +// global in a custom section is otherwise a valid link-time removal. +#define PJ_PLUGIN_DESCRIPTOR_KEEP __declspec(dllexport) +#elif defined(__APPLE__) +#define PJ_PLUGIN_DESCRIPTOR_SECTION_NAME "__PJ,__manifest" +#define PJ_PLUGIN_DESCRIPTOR_PLACEMENT __attribute__((section("__PJ,__manifest"))) +#define PJ_PLUGIN_DESCRIPTOR_KEEP __attribute__((visibility("default"), used)) +#else +#define PJ_PLUGIN_DESCRIPTOR_SECTION_NAME ".pj_manifest" +#define PJ_PLUGIN_DESCRIPTOR_PLACEMENT __attribute__((section(".pj_manifest"))) +// `used` keeps the compiler from dropping it; `retain` (SHF_GNU_RETAIN) keeps +// the linker from dropping it under --gc-sections. +#if defined(__has_attribute) +#if __has_attribute(retain) +#define PJ_PLUGIN_DESCRIPTOR_KEEP __attribute__((visibility("default"), used, retain)) +#endif +#endif +#ifndef PJ_PLUGIN_DESCRIPTOR_KEEP +#define PJ_PLUGIN_DESCRIPTOR_KEEP __attribute__((visibility("default"), used)) +#endif +#endif + +namespace PJ::detail { + +/// Family tags stored in the blob. Wire values — never renumber; the host's +/// PluginFamily mirrors them and static_asserts the correspondence. +enum PluginDescriptorFamily : uint32_t { + kDescriptorFamilyUnknown = 0, + kDescriptorFamilyDataSource = 1, + kDescriptorFamilyMessageParser = 2, + kDescriptorFamilyToolbox = 3, + kDescriptorFamilyDialog = 4, +}; + +/// Identifies a blob when scanning the raw section bytes. Not NUL-terminated. +inline constexpr char kPluginDescriptorMagic[8] = {'P', 'J', 'P', 'L', 'U', 'G', 'I', 'N'}; + +/// Bumped only if the header below changes shape. A reader that does not know +/// a blob's version skips that blob rather than misreading it. +inline constexpr uint32_t kPluginDescriptorBlobVersion = 1; + +/// Fixed-size prologue of every blob. All fields are written in the DSO's +/// native byte order; a reader parsing a foreign-endian image byte-swaps them +/// using the endianness declared by the container format's own header. +struct PluginDescriptorBlobHeader { + char magic[8]; + uint32_t blob_version; + /// Total bytes of this blob, header and trailing padding included. Lets a + /// reader walk a section holding several blobs without parsing each manifest. + uint32_t blob_size; + /// PJ_ABI_VERSION the plugin was compiled against. + uint32_t abi_version; + /// One of PluginDescriptorFamily. + uint32_t family; + /// Manifest length in bytes, excluding the NUL terminator. + uint32_t manifest_size; + uint32_t reserved; +}; +static_assert(sizeof(PluginDescriptorBlobHeader) == 32, "descriptor blob header is a wire format"); + +/// A header immediately followed by the manifest text. 8-byte alignment makes +/// every blob_size a multiple of 8, so consecutive blobs contributed by +/// different translation units stay walkable without gaps. +template +struct alignas(8) PluginDescriptorBlob { + PluginDescriptorBlobHeader header; + char manifest_json[JsonBytes]; +}; + +/// Length of a NUL-terminated manifest, excluding the terminator. +/// +/// Exists so the macro can size the blob exactly. Plugins spell their manifest +/// either as a `char[]` (the CMake-generated headers) or as a +/// `constexpr const char*` (hand-written ones), so the length cannot simply be +/// deduced from an array parameter. +constexpr std::size_t manifestLength(const char* manifest_json) { + std::size_t length = 0; + while (manifest_json[length] != '\0') { + ++length; + } + return length; +} + +/// Builds a blob from a NUL-terminated manifest. `JsonBytes` counts the +/// terminator, so it is manifestLength() + 1. +template +constexpr PluginDescriptorBlob makePluginDescriptorBlob(uint32_t family, const char* manifest_json) { + static_assert(JsonBytes >= 2, "manifest must be a non-empty NUL-terminated string"); + PluginDescriptorBlob blob{}; + for (std::size_t i = 0; i < sizeof(kPluginDescriptorMagic); ++i) { + blob.header.magic[i] = kPluginDescriptorMagic[i]; + } + blob.header.blob_version = kPluginDescriptorBlobVersion; + blob.header.blob_size = static_cast(sizeof(PluginDescriptorBlob)); + blob.header.abi_version = PJ_ABI_VERSION; + blob.header.family = family; + blob.header.manifest_size = static_cast(JsonBytes - 1); + blob.header.reserved = 0; + for (std::size_t i = 0; i < JsonBytes; ++i) { + blob.manifest_json[i] = manifest_json[i]; + } + return blob; +} + +} // namespace PJ::detail + +/// Emits one descriptor blob. `SymbolSuffix` keeps the names distinct when a +/// single DSO implements more than one family (a data source that also ships a +/// dialog, say). +/// +/// The variable is non-const for the same reason `pj_plugin_abi_version` is: a +/// namespace-scope `const` has internal linkage in C++, which MSVC then refuses +/// to place with __declspec(dllexport). Nothing writes to it. Being a plain +/// global rather than a vague-linkage entity, it never acquires STB_GNU_UNIQUE +/// binding itself. +/// +/// `constinit` is load-bearing, not decoration: the blob has to be present in +/// the image on disk. Without it, a manifest that is not a constant expression +/// would compile into a dynamic initialiser, leaving the section zero-filled on +/// disk and the whole static-discovery path silently reading nothing. It turns +/// that into a compile error instead. +#define PJ_EMBED_PLUGIN_DESCRIPTOR(SymbolSuffix, FamilyValue, ManifestJson) \ + extern "C" { \ + PJ_PLUGIN_DESCRIPTOR_KEEP PJ_PLUGIN_DESCRIPTOR_PLACEMENT constinit auto pj_plugin_descriptor_##SymbolSuffix = \ + PJ::detail::makePluginDescriptorBlob(FamilyValue, ManifestJson); \ + } + +// Statically linked builds have no DSO to inspect, and one blob symbol per +// family would collide across the plugins folded into the host binary. +#ifdef PJ_STATIC_PLUGINS +#undef PJ_EMBED_PLUGIN_DESCRIPTOR +#define PJ_EMBED_PLUGIN_DESCRIPTOR(SymbolSuffix, FamilyValue, ManifestJson) +#endif + +#endif // PJ_PLUGIN_DESCRIPTOR_SECTION_HPP diff --git a/pj_base/include/pj_base/sdk/data_source_plugin_base.hpp b/pj_base/include/pj_base/sdk/data_source_plugin_base.hpp index 2da80418..acd4d3cf 100644 --- a/pj_base/include/pj_base/sdk/data_source_plugin_base.hpp +++ b/pj_base/include/pj_base/sdk/data_source_plugin_base.hpp @@ -37,6 +37,7 @@ #include "pj_base/data_source_protocol.h" #include "pj_base/expected.hpp" #include "pj_base/plugin_abi_export.hpp" +#include "pj_base/plugin_descriptor_section.hpp" #include "pj_base/sdk/data_source_host_views.hpp" #include "pj_base/sdk/plugin_data_api.hpp" #include "pj_base/sdk/service_registry.hpp" @@ -227,7 +228,8 @@ class DataSourcePluginBase { }, \ manifest); \ return vt; \ - } + } \ + PJ_EMBED_PLUGIN_DESCRIPTOR(data_source, PJ::detail::kDescriptorFamilyDataSource, manifest) // Variant for namespaced plugin classes. SymbolName must be an unqualified // identifier and is used only to form the unique static getter name. diff --git a/pj_base/include/pj_base/sdk/toolbox_plugin_base.hpp b/pj_base/include/pj_base/sdk/toolbox_plugin_base.hpp index 59dc3240..4fb2907b 100644 --- a/pj_base/include/pj_base/sdk/toolbox_plugin_base.hpp +++ b/pj_base/include/pj_base/sdk/toolbox_plugin_base.hpp @@ -17,6 +17,7 @@ #include "pj_base/expected.hpp" #include "pj_base/plugin_abi_export.hpp" +#include "pj_base/plugin_descriptor_section.hpp" #include "pj_base/sdk/data_source_host_views.hpp" // ParserIngestHostView, errorToString #include "pj_base/sdk/plugin_data_api.hpp" #include "pj_base/sdk/service_registry.hpp" @@ -251,7 +252,8 @@ class ToolboxPluginBase { }, \ manifest); \ return vt; \ - } + } \ + PJ_EMBED_PLUGIN_DESCRIPTOR(toolbox, PJ::detail::kDescriptorFamilyToolbox, manifest) // Variant for namespaced plugin classes. SymbolName must be an unqualified // identifier and is used only to form the unique static getter name. diff --git a/pj_plugins/CLAUDE.md b/pj_plugins/CLAUDE.md index 731a87f6..04116808 100644 --- a/pj_plugins/CLAUDE.md +++ b/pj_plugins/CLAUDE.md @@ -24,7 +24,9 @@ submodule-internal modules; `pj_base` carries none). - `include/pj_plugins/testing/` — `ToolboxTestStore` (fake Arrow host for tests). - `dialog_protocol/` — **nested module** (own CMake): the Dialog C ABI, C++ dialog SDK, and host dialog loader/handle. See `dialog_protocol/CLAUDE.md`. -- `src/` — loader/catalog `.cpp`; `src/detail/` vtable validation + dlopen. +- `src/` — loader/catalog `.cpp`; `src/detail/` vtable validation, dlopen, and + the object-file reader that pulls a plugin's descriptor section off disk so + discovery never has to map the DSO (`descriptor_section_reader.hpp`). - `examples/` — mock plugins exercised by tests (`mock_data_source`, …). - `tests/` — host-side loader + lifecycle tests. diff --git a/pj_plugins/CMakeLists.txt b/pj_plugins/CMakeLists.txt index b30c3689..e5c40329 100644 --- a/pj_plugins/CMakeLists.txt +++ b/pj_plugins/CMakeLists.txt @@ -1,6 +1,7 @@ find_package(nlohmann_json REQUIRED) add_library(pj_plugin_loader_detail STATIC + src/detail/descriptor_section_reader.cpp src/detail/vtable_validation.cpp ) target_include_directories(pj_plugin_loader_detail PUBLIC @@ -362,6 +363,7 @@ target_compile_definitions(plugin_catalog_test PRIVATE PJ_MISSING_ID_PLUGIN_PATH="$" PJ_INVALID_OPTIONAL_PLUGIN_PATH="$" PJ_MISSING_REQUIRED_SLOTS_PLUGIN_PATH="$" + PJ_MOCK_SOURCE_WITH_DIALOG_PLUGIN_PATH="$" ) target_compile_options(plugin_catalog_test PRIVATE ${PJ_WARNING_FLAGS}) target_link_libraries(plugin_catalog_test PRIVATE @@ -372,9 +374,19 @@ add_dependencies(plugin_catalog_test mock_data_source_plugin mock_toolbox_plugin mock_dialog_plugin missing_id_data_source_plugin invalid_optional_manifest_data_source_plugin missing_required_slots_plugin static_manifest_dialog_plugin legacy_macro_dialog_plugin - missing_dialog_required_slots_plugin) + missing_dialog_required_slots_plugin mock_source_with_dialog_plugin) add_test(NAME plugin_catalog_test COMMAND plugin_catalog_test) +# Unit test: object-file parsing for the static plugin-descriptor reader. +# Builds synthetic ELF / PE / Mach-O containers, so it needs no fixture DSOs and +# covers the formats this host cannot itself produce. +add_executable(descriptor_section_reader_test tests/descriptor_section_reader_test.cpp) +target_compile_options(descriptor_section_reader_test PRIVATE ${PJ_WARNING_FLAGS}) +target_link_libraries(descriptor_section_reader_test PRIVATE + pj_plugin_loader_detail pj_base GTest::gtest_main +) +add_test(NAME descriptor_section_reader_test COMMAND descriptor_section_reader_test) + endif() # PJ_BUILD_TESTS # --------------------------------------------------------------------------- diff --git a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_base.hpp b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_base.hpp index 0843ae0c..dc0b7034 100644 --- a/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_base.hpp +++ b/pj_plugins/dialog_protocol/include/pj_plugins/sdk/dialog_plugin_base.hpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -287,8 +288,13 @@ PJ_borrowed_dialog_t borrowDialog(DialogT& dialog) noexcept { #define PJ_DIALOG_PLUGIN(...) \ PJ_DIALOG_PLUGIN_EXPAND( \ PJ_DIALOG_PLUGIN_SELECT(__VA_ARGS__, PJ_DIALOG_PLUGIN_WITH_MANIFEST, PJ_DIALOG_PLUGIN_LEGACY)(__VA_ARGS__)) -#define PJ_DIALOG_PLUGIN_LEGACY(ClassName) PJ_DIALOG_PLUGIN_WITH_MANIFEST(ClassName, nullptr) -#define PJ_DIALOG_PLUGIN_WITH_MANIFEST(ClassName, ManifestJson) \ +// A dialog declared without a manifest has nothing to embed, so it emits no +// descriptor blob and discovery falls back to loading the DSO. +#define PJ_DIALOG_PLUGIN_LEGACY(ClassName) PJ_DIALOG_PLUGIN_IMPL(ClassName, nullptr) +#define PJ_DIALOG_PLUGIN_WITH_MANIFEST(ClassName, ManifestJson) \ + PJ_DIALOG_PLUGIN_IMPL(ClassName, ManifestJson) \ + PJ_EMBED_PLUGIN_DESCRIPTOR(dialog, PJ::detail::kDescriptorFamilyDialog, ManifestJson) +#define PJ_DIALOG_PLUGIN_IMPL(ClassName, ManifestJson) \ PJ_EXPORT_PLUGIN_ABI_VERSION(PJ_DIALOG_EXPORT) \ extern "C" PJ_DIALOG_EXPORT const PJ_dialog_vtable_t* PJ_get_dialog_vtable() noexcept { \ static const PJ_dialog_vtable_t* vt = PJ::DialogPluginBase::vtableWithCreate( \ @@ -349,4 +355,9 @@ PJ_borrowed_dialog_t borrowDialog(DialogT& dialog) noexcept { } #define PJ_DIALOG_PLUGIN_WITH_MANIFEST(ClassName, ManifestJson) \ PJ_DIALOG_PLUGIN_NAMED(ClassName, ClassName, ManifestJson) +// The manifest-less form has to be rerouted too: its shared body emits the +// fixed `extern "C" PJ_get_dialog_vtable`, which collides across the plugins +// folded into one statically linked binary. +#undef PJ_DIALOG_PLUGIN_LEGACY +#define PJ_DIALOG_PLUGIN_LEGACY(ClassName) PJ_DIALOG_PLUGIN_NAMED(ClassName, ClassName, nullptr) #endif // PJ_STATIC_PLUGINS diff --git a/pj_plugins/docs/ARCHITECTURE.md b/pj_plugins/docs/ARCHITECTURE.md index c645cc5f..ed240407 100644 --- a/pj_plugins/docs/ARCHITECTURE.md +++ b/pj_plugins/docs/ARCHITECTURE.md @@ -132,16 +132,34 @@ previously-circulated pre-v4 design included): the ABI headers carries a `[main-thread]` / `[stream-thread]` / `[thread-safe]` comment. Host-side runtime checking is optional (reserved for a future `"pj.thread_check.v1"` service). -- **Embedded-manifest plugin discovery.** Each DSO exports a - family-specific protocol vtable with embedded metadata (`manifest_json` - for data sources, parsers, toolboxes, and newly built dialogs; legacy v4.0 - dialogs fall back to `create()` + `get_manifest()` during inspection). +- **Embedded-manifest plugin discovery, without loading the plugin.** Every + DSO built with a `PJ_*_PLUGIN` macro carries a self-describing descriptor + blob — magic, ABI version, family, manifest JSON — in a dedicated section + (`.pj_manifest` on ELF, `.pjmani` on PE, `__PJ,__manifest` on Mach-O). See + `pj_base/plugin_descriptor_section.hpp` for the layout. Host-side `PJ::scanPluginDsos(dir)` (in - `pj_plugins/host/plugin_catalog.hpp`) walks platform plugin libraries, - loads each candidate, validates the ABI and protocol vtable, and parses - `id`, `name`, `version`, family-specific fields, and optional metadata - directly from the embedded manifest. Broken or incompatible candidates - are reported as diagnostics while discovery continues. + `pj_plugins/host/plugin_catalog.hpp`) walks platform plugin libraries and + reads that section straight off disk, parsing `id`, `name`, `version`, + family-specific fields, and optional metadata without ever mapping the + image. Broken or incompatible candidates are reported as diagnostics while + discovery continues; an ABI-version mismatch is rejected from the section + alone, so a plugin built against a different ABI is never loaded. + + Discovery must not `dlopen`, because **`dlclose` cannot undo it**: glibc + pins a library NODELETE as soon as it defines an `STB_GNU_UNIQUE` symbol, + which any vague-linkage static produces. An inspect-then-close pass leaves + the plugin resident for the life of the process, and a second copy of the + same plugin loaded later from another path binds to the first copy's + storage — with its initialisation guards already set — so layout drift + between the two builds corrupts the process. + + A DSO with no descriptor section (built against an older SDK, or with a + hand-written vtable) falls back to the original path: `dlopen`, validate + the ABI and protocol vtable, read `manifest_json` from the vtable, and + `dlclose`. Legacy v4.0 dialogs additionally fall back to `create()` + + `get_manifest()`. Vtable-shape validation for plugins taking the static + path happens when they are actually loaded, which the family loaders + (`data_source_library.cpp` and siblings) do for every plugin regardless. - **No more RTLD_DEEPBIND.** The loader uses `RTLD_NOW | RTLD_LOCAL` only (DEEPBIND was a documented ASAN/allocator-interposition trap). Plugin-local symbol isolation is left to `-fvisibility=hidden`. @@ -266,6 +284,7 @@ pj_plugins/ toolbox_library.hpp toolbox_handle.hpp plugin_catalog.hpp ← embedded-manifest DSO scanner (scanPluginDsos / inspectPluginDso) + ../src/detail/descriptor_section_reader.hpp ← reads the descriptor section off disk (ELF / PE / Mach-O) service_registry_builder.hpp ← service wiring into bind() config_envelope.hpp ← versioned config wrapper include/pj_plugins/sdk/ diff --git a/pj_plugins/include/pj_plugins/sdk/message_parser_plugin_base.hpp b/pj_plugins/include/pj_plugins/sdk/message_parser_plugin_base.hpp index 0c06b981..d101b804 100644 --- a/pj_plugins/include/pj_plugins/sdk/message_parser_plugin_base.hpp +++ b/pj_plugins/include/pj_plugins/sdk/message_parser_plugin_base.hpp @@ -27,6 +27,7 @@ #include "pj_base/expected.hpp" #include "pj_base/message_parser_protocol.h" #include "pj_base/plugin_abi_export.hpp" +#include "pj_base/plugin_descriptor_section.hpp" #include "pj_base/sdk/plugin_data_api.hpp" #include "pj_base/sdk/service_registry.hpp" #include "pj_base/sdk/service_traits.hpp" @@ -375,7 +376,8 @@ class MessageParserPluginBase { }, \ manifest); \ return vt; \ - } + } \ + PJ_EMBED_PLUGIN_DESCRIPTOR(message_parser, PJ::detail::kDescriptorFamilyMessageParser, manifest) // Variant for namespaced plugin classes. SymbolName must be an unqualified // identifier and is used only to form the unique static getter name. diff --git a/pj_plugins/src/detail/descriptor_section_reader.cpp b/pj_plugins/src/detail/descriptor_section_reader.cpp new file mode 100644 index 00000000..a025ae31 --- /dev/null +++ b/pj_plugins/src/detail/descriptor_section_reader.cpp @@ -0,0 +1,487 @@ +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +#include "detail/descriptor_section_reader.hpp" + +#include +#include +#include +#include +#include +#include +#include + +#include "pj_base/plugin_descriptor_section.hpp" + +// Object-file parsing, deliberately hand-rolled against fixed field offsets +// rather than the platform's own headers (, , +// ). Two reasons: every format must compile on every host so +// the tests can exercise all three, and reading fields by offset sidesteps +// struct-packing and type-width differences between toolchains. +// +// Only what is needed to locate one named section is implemented. Anything not +// understood becomes an error, which the caller turns into a dlopen fallback — +// so being conservative here costs discovery speed, never correctness. + +namespace PJ::detail { +namespace { + +/// Copies `len` bytes at `offset` out of the image. False if the range is out +/// of bounds or unreadable, leaving `dst` untouched. +using ReadBytes = std::function; + +/// Bounds-checked scalar reads over an image, in the image's byte order. +class FieldReader { + public: + explicit FieldReader(const ReadBytes& read) : read_(read) {} + + void setBigEndian(bool big_endian) { + big_endian_ = big_endian; + } + + [[nodiscard]] std::optional u16(uint64_t offset) const { + return scalar(offset); + } + [[nodiscard]] std::optional u32(uint64_t offset) const { + return scalar(offset); + } + [[nodiscard]] std::optional u64(uint64_t offset) const { + return scalar(offset); + } + + /// Reads a 32- or 64-bit field as a 64-bit value, for the offset/size fields + /// that differ in width between the 32- and 64-bit flavour of a format. + [[nodiscard]] std::optional word(uint64_t offset, bool wide) const { + if (wide) { + return u64(offset); + } + const auto narrow = u32(offset); + return narrow ? std::optional(*narrow) : std::nullopt; + } + + /// Reads a fixed-width, NUL-padded name field (Mach-O segment and section + /// names, PE section names). + [[nodiscard]] std::optional fixedName(uint64_t offset, size_t width) const { + std::string raw(width, '\0'); + if (!read_(offset, width, raw.data())) { + return std::nullopt; + } + const auto terminator = raw.find('\0'); + if (terminator != std::string::npos) { + raw.resize(terminator); + } + return raw; + } + + [[nodiscard]] bool bytes(uint64_t offset, size_t len, void* dst) const { + return read_(offset, len, dst); + } + + private: + template + [[nodiscard]] std::optional scalar(uint64_t offset) const { + std::array raw{}; + if (!read_(offset, sizeof(T), raw.data())) { + return std::nullopt; + } + T value = 0; + for (size_t i = 0; i < sizeof(T); ++i) { + const size_t index = big_endian_ ? i : sizeof(T) - 1 - i; + value = static_cast((value << 8) | raw[index]); + } + return value; + } + + const ReadBytes& read_; + bool big_endian_ = false; +}; + +/// File offset and byte count of the descriptor section inside an image. +struct SectionRange { + uint64_t offset = 0; + uint64_t size = 0; +}; + +/// Absent value means "parsed fine, no such section" — distinct from an error, +/// which means the container could not be parsed at all. +using FoundSection = Expected>; + +// --- Blob walking ------------------------------------------------------------ + +/// Blobs are 8-byte aligned by construction, but the linker leaves padding +/// between the contributions of different translation units, so the section is +/// scanned for the magic rather than walked blob_size to blob_size. +constexpr uint64_t kBlobScanStride = 8; + +/// Largest manifest accepted. Guards against a corrupt length turning into a +/// multi-gigabyte allocation; real manifests are well under a kilobyte. +constexpr uint32_t kMaxManifestSize = 1u << 20; + +[[nodiscard]] std::optional readBlobAt( + const FieldReader& reader, uint64_t blob_offset, uint64_t section_end) { + std::array magic{}; + if (!reader.bytes(blob_offset, magic.size(), magic.data())) { + return std::nullopt; + } + if (std::memcmp(magic.data(), kPluginDescriptorMagic, magic.size()) != 0) { + return std::nullopt; + } + + const auto blob_version = reader.u32(blob_offset + 8); + const auto abi_version = reader.u32(blob_offset + 16); + const auto family = reader.u32(blob_offset + 20); + const auto manifest_size = reader.u32(blob_offset + 24); + if (!blob_version || !abi_version || !family || !manifest_size) { + return std::nullopt; + } + // A blob written by a newer SDK may have a different shape; skipping it is + // the only safe reading. + if (*blob_version != kPluginDescriptorBlobVersion) { + return std::nullopt; + } + if (*manifest_size == 0 || *manifest_size > kMaxManifestSize) { + return std::nullopt; + } + + const uint64_t manifest_offset = blob_offset + sizeof(PluginDescriptorBlobHeader); + if (manifest_offset + *manifest_size > section_end) { + return std::nullopt; + } + + EmbeddedDescriptor descriptor; + descriptor.abi_version = *abi_version; + descriptor.family = *family; + descriptor.manifest_json.resize(*manifest_size); + if (!reader.bytes(manifest_offset, *manifest_size, descriptor.manifest_json.data())) { + return std::nullopt; + } + return descriptor; +} + +[[nodiscard]] std::vector readBlobs(const FieldReader& reader, const SectionRange& section) { + std::vector descriptors; + const uint64_t end = section.offset + section.size; + for (uint64_t at = section.offset; at + sizeof(PluginDescriptorBlobHeader) <= end; at += kBlobScanStride) { + if (auto descriptor = readBlobAt(reader, at, end)) { + descriptors.push_back(std::move(*descriptor)); + } + } + return descriptors; +} + +// --- ELF --------------------------------------------------------------------- + +constexpr uint32_t kElfSectionTypeNoBits = 8; // SHT_NOBITS: occupies no file bytes +constexpr uint16_t kElfSectionIndexXindex = 0xFFFF; +constexpr uint8_t kElfClass32 = 1; +constexpr uint8_t kElfClass64 = 2; +constexpr uint8_t kElfDataLittleEndian = 1; +constexpr uint8_t kElfDataBigEndian = 2; + +struct ElfShape { + bool is_64 = false; + uint64_t section_header_offset = 0; + uint16_t section_header_size = 0; + uint64_t section_count = 0; + uint64_t string_table_index = 0; +}; + +struct ElfSectionHeader { + uint32_t name_offset = 0; + uint32_t type = 0; + uint64_t file_offset = 0; + uint64_t size = 0; +}; + +[[nodiscard]] std::optional readElfSectionHeader( + const FieldReader& reader, const ElfShape& shape, uint64_t index) { + const uint64_t at = shape.section_header_offset + index * shape.section_header_size; + const auto name_offset = reader.u32(at); + const auto type = reader.u32(at + 4); + // sh_offset and sh_size sit at different offsets in the two ELF classes. + const auto file_offset = reader.word(at + (shape.is_64 ? 24 : 16), shape.is_64); + const auto size = reader.word(at + (shape.is_64 ? 32 : 20), shape.is_64); + if (!name_offset || !type || !file_offset || !size) { + return std::nullopt; + } + return ElfSectionHeader{*name_offset, *type, *file_offset, *size}; +} + +[[nodiscard]] FoundSection findElfSection(FieldReader& reader, std::string_view wanted) { + // EI_CLASS and EI_DATA are position-defined bytes, not endian-encoded values. + std::array ident{}; + if (!reader.bytes(4, ident.size(), ident.data())) { + return unexpected(std::string("ELF identification is truncated")); + } + if (ident[0] != kElfClass32 && ident[0] != kElfClass64) { + return unexpected(std::string("unsupported ELF class")); + } + if (ident[1] != kElfDataLittleEndian && ident[1] != kElfDataBigEndian) { + return unexpected(std::string("unsupported ELF data encoding")); + } + reader.setBigEndian(ident[1] == kElfDataBigEndian); + + ElfShape shape; + shape.is_64 = ident[0] == kElfClass64; + + const auto section_header_offset = reader.word(shape.is_64 ? 0x28 : 0x20, shape.is_64); + const auto section_header_size = reader.u16(shape.is_64 ? 0x3A : 0x2E); + const auto section_count = reader.u16(shape.is_64 ? 0x3C : 0x30); + const auto string_table_index = reader.u16(shape.is_64 ? 0x3E : 0x32); + if (!section_header_offset || !section_header_size || !section_count || !string_table_index) { + return unexpected(std::string("ELF header is truncated")); + } + if (*section_header_offset == 0 || *section_header_size == 0) { + return std::optional{}; // stripped of its section table + } + shape.section_header_offset = *section_header_offset; + shape.section_header_size = *section_header_size; + shape.section_count = *section_count; + shape.string_table_index = *string_table_index; + + // Section counts and name-table indices too large for the 16-bit header + // fields escape through section 0. The two escapes are independent. + if (shape.section_count == 0) { + const auto zero = readElfSectionHeader(reader, shape, 0); + if (!zero) { + return unexpected(std::string("ELF section table is truncated")); + } + shape.section_count = zero->size; + } + if (shape.string_table_index == kElfSectionIndexXindex) { + const auto link = reader.u32(shape.section_header_offset + (shape.is_64 ? 40 : 24)); + if (!link) { + return unexpected(std::string("ELF section table is truncated")); + } + shape.string_table_index = *link; + } + if (shape.string_table_index >= shape.section_count) { + return unexpected(std::string("ELF section name table index is out of range")); + } + + const auto string_table = readElfSectionHeader(reader, shape, shape.string_table_index); + if (!string_table) { + return unexpected(std::string("ELF section name table is unreadable")); + } + + // Compared with its terminator included, so ".pj_manifest2" cannot match. + std::string candidate(wanted.size() + 1, '\0'); + for (uint64_t index = 0; index < shape.section_count; ++index) { + const auto header = readElfSectionHeader(reader, shape, index); + if (!header) { + return unexpected(std::string("ELF section table is truncated")); + } + if (header->type == kElfSectionTypeNoBits || header->size == 0) { + continue; + } + if (header->name_offset >= string_table->size) { + continue; + } + if (!reader.bytes(string_table->file_offset + header->name_offset, candidate.size(), candidate.data())) { + continue; + } + if (candidate.compare(0, wanted.size(), wanted) == 0 && candidate[wanted.size()] == '\0') { + return std::optional({header->file_offset, header->size}); + } + } + return std::optional{}; +} + +// --- PE ---------------------------------------------------------------------- + +constexpr uint64_t kPeSectionHeaderSize = 40; +constexpr size_t kPeSectionNameSize = 8; +constexpr uint64_t kPeCoffHeaderSize = 20; + +[[nodiscard]] FoundSection findPeSection(const FieldReader& reader, std::string_view wanted) { + const auto nt_offset = reader.u32(0x3C); + if (!nt_offset) { + return unexpected(std::string("PE header is truncated")); + } + std::array signature{}; + if (!reader.bytes(*nt_offset, signature.size(), signature.data())) { + return unexpected(std::string("PE header is truncated")); + } + if (signature[0] != 'P' || signature[1] != 'E' || signature[2] != '\0' || signature[3] != '\0') { + return unexpected(std::string("missing PE signature")); + } + + const uint64_t coff = *nt_offset + 4; + const auto section_count = reader.u16(coff + 2); + const auto optional_header_size = reader.u16(coff + 16); + if (!section_count || !optional_header_size) { + return unexpected(std::string("PE header is truncated")); + } + + const uint64_t table = coff + kPeCoffHeaderSize + *optional_header_size; + for (uint16_t index = 0; index < *section_count; ++index) { + const uint64_t at = table + index * kPeSectionHeaderSize; + const auto name = reader.fixedName(at, kPeSectionNameSize); + if (!name) { + return unexpected(std::string("PE section table is truncated")); + } + if (*name != wanted) { + continue; + } + const auto virtual_size = reader.u32(at + 8); + const auto raw_size = reader.u32(at + 16); + const auto raw_offset = reader.u32(at + 20); + if (!virtual_size || !raw_size || !raw_offset) { + return unexpected(std::string("PE section table is truncated")); + } + // SizeOfRawData is rounded up to the file alignment, so the payload ends at + // VirtualSize whenever that is the smaller of the two. + const uint64_t size = *virtual_size == 0 ? *raw_size : std::min(*virtual_size, *raw_size); + return std::optional({*raw_offset, size}); + } + return std::optional{}; +} + +// --- Mach-O ------------------------------------------------------------------ + +constexpr uint32_t kMachOMagic64 = 0xFEEDFACFu; +constexpr uint32_t kMachOMagic32 = 0xFEEDFACEu; +constexpr uint32_t kMachOCommandSegment32 = 0x01; +constexpr uint32_t kMachOCommandSegment64 = 0x19; +constexpr size_t kMachOFixedNameSize = 16; + +/// `wanted` is the "SEGMENT,SECTION" pair, spelled as the section attribute +/// spells it. +[[nodiscard]] FoundSection findMachOSection(const FieldReader& reader, bool is_64, std::string_view wanted) { + const auto comma = wanted.find(','); + if (comma == std::string_view::npos) { + return unexpected(std::string("Mach-O section name must be SEGMENT,SECTION")); + } + const std::string_view wanted_segment = wanted.substr(0, comma); + const std::string_view wanted_section = wanted.substr(comma + 1); + + const auto command_count = reader.u32(16); + if (!command_count) { + return unexpected(std::string("Mach-O header is truncated")); + } + + uint64_t at = is_64 ? 32 : 28; + for (uint32_t index = 0; index < *command_count; ++index) { + const auto command = reader.u32(at); + const auto command_size = reader.u32(at + 4); + if (!command || !command_size || *command_size == 0) { + return unexpected(std::string("Mach-O load commands are truncated")); + } + const bool segment_64 = *command == kMachOCommandSegment64; + const bool segment_32 = *command == kMachOCommandSegment32; + if (segment_64 || segment_32) { + const auto segment_name = reader.fixedName(at + 8, kMachOFixedNameSize); + const auto section_count = reader.u32(at + (segment_64 ? 64 : 48)); + if (!segment_name || !section_count) { + return unexpected(std::string("Mach-O load commands are truncated")); + } + const uint64_t sections = at + (segment_64 ? 72 : 56); + const uint64_t section_stride = segment_64 ? 80 : 68; + for (uint32_t section = 0; section < *section_count; ++section) { + const uint64_t section_at = sections + section * section_stride; + const auto section_name = reader.fixedName(section_at, kMachOFixedNameSize); + if (!section_name) { + return unexpected(std::string("Mach-O section table is truncated")); + } + if (*section_name != wanted_section || *segment_name != wanted_segment) { + continue; + } + const auto size = reader.word(section_at + (segment_64 ? 40 : 36), segment_64); + const auto file_offset = reader.u32(section_at + (segment_64 ? 48 : 40)); + if (!size || !file_offset) { + return unexpected(std::string("Mach-O section table is truncated")); + } + return std::optional({*file_offset, *size}); + } + } + at += *command_size; + } + return std::optional{}; +} + +// --- Dispatch ---------------------------------------------------------------- + +[[nodiscard]] constexpr uint32_t byteSwap32(uint32_t value) { + return ((value & 0x000000FFu) << 24) | ((value & 0x0000FF00u) << 8) | ((value & 0x00FF0000u) >> 8) | + ((value & 0xFF000000u) >> 24); +} + +[[nodiscard]] FoundSection findDescriptorSection(FieldReader& reader, const std::array& head) { + if (head[0] == 0x7F && head[1] == 'E' && head[2] == 'L' && head[3] == 'F') { + return findElfSection(reader, PJ_PLUGIN_DESCRIPTOR_SECTION_NAME_ELF); + } + if (head[0] == 'M' && head[1] == 'Z') { + return findPeSection(reader, PJ_PLUGIN_DESCRIPTOR_SECTION_NAME_PE); + } + + const uint32_t magic = static_cast(head[0]) | (static_cast(head[1]) << 8) | + (static_cast(head[2]) << 16) | (static_cast(head[3]) << 24); + const uint32_t swapped = byteSwap32(magic); + const bool native = magic == kMachOMagic64 || magic == kMachOMagic32; + const bool foreign = swapped == kMachOMagic64 || swapped == kMachOMagic32; + if (!native && !foreign) { + // Universal ("fat") Mach-O archives land here too: they are not parsed, and + // the caller falls back to dlopen. + return unexpected(std::string("unrecognised object file format")); + } + reader.setBigEndian(foreign); + return findMachOSection(reader, (native ? magic : swapped) == kMachOMagic64, PJ_PLUGIN_DESCRIPTOR_SECTION_NAME_MACHO); +} + +[[nodiscard]] Expected> readDescriptors(const ReadBytes& read) { + FieldReader reader(read); + std::array head{}; + if (!read(0, head.size(), head.data())) { + return unexpected(std::string("image is too small to identify")); + } + + auto section = findDescriptorSection(reader, head); + if (!section) { + return unexpected(section.error()); + } + if (!section->has_value()) { + return std::vector{}; + } + return readBlobs(reader, **section); +} + +} // namespace + +Expected> readEmbeddedDescriptors(const std::filesystem::path& dso_path) { + std::error_code ec; + const auto file_size = static_cast(std::filesystem::file_size(dso_path, ec)); + if (ec) { + return unexpected("cannot size " + dso_path.string() + ": " + ec.message()); + } + std::ifstream file(dso_path, std::ios::binary); + if (!file) { + return unexpected("cannot open " + dso_path.string()); + } + + const ReadBytes read = [&file, file_size](uint64_t offset, size_t len, void* dst) { + if (len == 0 || offset > file_size || len > file_size - offset) { + return false; + } + file.clear(); + file.seekg(static_cast(offset), std::ios::beg); + if (!file) { + return false; + } + file.read(static_cast(dst), static_cast(len)); + return static_cast(file); + }; + return readDescriptors(read); +} + +Expected> readEmbeddedDescriptors(std::span image) { + const ReadBytes read = [image](uint64_t offset, size_t len, void* dst) { + if (len == 0 || offset > image.size() || len > image.size() - offset) { + return false; + } + std::memcpy(dst, image.data() + static_cast(offset), len); + return true; + }; + return readDescriptors(read); +} + +} // namespace PJ::detail diff --git a/pj_plugins/src/detail/descriptor_section_reader.hpp b/pj_plugins/src/detail/descriptor_section_reader.hpp new file mode 100644 index 00000000..664aacf8 --- /dev/null +++ b/pj_plugins/src/detail/descriptor_section_reader.hpp @@ -0,0 +1,43 @@ +#pragma once +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 + +#include +#include +#include +#include +#include +#include + +#include "pj_base/expected.hpp" + +namespace PJ::detail { + +/// One descriptor blob recovered from a DSO's plugin-descriptor section. +struct EmbeddedDescriptor { + /// PJ_ABI_VERSION the plugin was compiled against. Reported rather than + /// enforced here: the caller decides whether a mismatch rejects the plugin. + uint32_t abi_version = 0; + /// A PJ::detail::PluginDescriptorFamily value. + uint32_t family = 0; + std::string manifest_json; +}; + +/// Extracts every descriptor blob from a plugin DSO on disk without loading it, +/// so no plugin code runs and the process never maps the image. +/// +/// Only the container headers and the descriptor section are read — inspecting +/// a 50 MB plugin costs a few KB, not a full slurp. +/// +/// An empty vector means the image parsed cleanly but carries no descriptor +/// section: a plugin built before the section existed, or one with a +/// hand-written vtable. Callers treat that as "fall back to dlopen", which is +/// why it is a value and not an error. An error means the container itself +/// could not be read (missing or truncated file, unsupported format). +[[nodiscard]] Expected> readEmbeddedDescriptors(const std::filesystem::path& dso_path); + +/// Same, over an image already in memory. Used by the format tests, which build +/// synthetic ELF/PE/Mach-O containers rather than shipping binary fixtures. +[[nodiscard]] Expected> readEmbeddedDescriptors(std::span image); + +} // namespace PJ::detail diff --git a/pj_plugins/src/plugin_catalog.cpp b/pj_plugins/src/plugin_catalog.cpp index aff6f50f..98725fc5 100644 --- a/pj_plugins/src/plugin_catalog.cpp +++ b/pj_plugins/src/plugin_catalog.cpp @@ -6,18 +6,22 @@ #include #include +#include #include #include #include +#include #include #include #include #include +#include "detail/descriptor_section_reader.hpp" #include "detail/library_loader.hpp" #include "detail/vtable_validation.hpp" #include "pj_base/data_source_protocol.h" #include "pj_base/message_parser_protocol.h" +#include "pj_base/plugin_descriptor_section.hpp" #include "pj_base/toolbox_protocol.h" #include "pj_plugins/dialog_protocol.h" @@ -161,6 +165,47 @@ Expected findEmbeddedManifest(void* handle) { return unexpected(out.str()); } +// Probe order for a DSO that implements several families. Mirrors +// findEmbeddedManifest's sequence so static and dlopen discovery agree on which +// family a multi-family DSO is reported as. +constexpr std::array kFamilyPrecedence = { + PluginFamily::kDataSource, PluginFamily::kMessageParser, PluginFamily::kToolbox, PluginFamily::kDialog}; + +static_assert(static_cast(PluginFamily::kDataSource) == detail::kDescriptorFamilyDataSource); +static_assert(static_cast(PluginFamily::kMessageParser) == detail::kDescriptorFamilyMessageParser); +static_assert(static_cast(PluginFamily::kToolbox) == detail::kDescriptorFamilyToolbox); +static_assert(static_cast(PluginFamily::kDialog) == detail::kDescriptorFamilyDialog); + +/// Recovers the manifest from the DSO's descriptor section, without loading it. +/// +/// An absent value means "no usable descriptor section, fall back to dlopen" — +/// an old plugin, a hand-written vtable, or an image this build cannot parse. +/// An error means the plugin must be rejected outright, which currently covers +/// only an ABI mismatch: loading a DSO built against a different ABI is exactly +/// what the boot handshake exists to prevent, so it must not fall through to a +/// path that would dlopen it. +Expected> staticManifestCandidate(const std::filesystem::path& dso_path) { + auto descriptors = detail::readEmbeddedDescriptors(dso_path); + if (!descriptors || descriptors->empty()) { + return std::optional{}; + } + for (const detail::EmbeddedDescriptor& descriptor : *descriptors) { + if (descriptor.abi_version != PJ_ABI_VERSION) { + return unexpected( + fmt::format( + "plugin pj_plugin_abi_version mismatch (expected {}, got {})", PJ_ABI_VERSION, descriptor.abi_version)); + } + } + for (PluginFamily family : kFamilyPrecedence) { + for (const detail::EmbeddedDescriptor& descriptor : *descriptors) { + if (descriptor.family == static_cast(family)) { + return std::optional({family, descriptor.manifest_json}); + } + } + } + return std::optional{}; // only families this build does not know +} + Expected> readStringArray(const nlohmann::json& j, std::string_view key) { std::vector values; const auto it = j.find(std::string(key)); @@ -299,6 +344,25 @@ Expected inspectPluginDso(const std::filesystem::path& dso_pat } auto with_path = [&](const std::string& error) { return fmt::format("{}: {}", dso_path.string(), error); }; + // Preferred path: read the descriptor straight out of the image on disk. It + // is not just faster — it is the only way to inspect a DSO without leaving it + // resident, because dlclose does not unmap a library that defines + // STB_GNU_UNIQUE symbols. See pj_base/plugin_descriptor_section.hpp. + auto embedded = staticManifestCandidate(dso_path); + if (!embedded) { + return unexpected(with_path(embedded.error())); + } + if (embedded->has_value()) { + auto descriptor = decodeManifest(dso_path, (*embedded)->family, (*embedded)->manifest_json); + if (!descriptor) { + return unexpected(with_path(descriptor.error())); + } + return *descriptor; + } + + // Fallback for a DSO with no descriptor section — built against an SDK that + // predates it, or carrying a hand-written vtable. Vtable-shape validation + // runs here for those; every plugin is validated again when it is loaded. auto handle = detail::loadLibraryHandle(dso_path.string()); if (!handle) { return unexpected(with_path(handle.error())); diff --git a/pj_plugins/tests/descriptor_section_reader_test.cpp b/pj_plugins/tests/descriptor_section_reader_test.cpp new file mode 100644 index 00000000..62c096d9 --- /dev/null +++ b/pj_plugins/tests/descriptor_section_reader_test.cpp @@ -0,0 +1,497 @@ +// Copyright 2026 Davide Faconti +// SPDX-License-Identifier: Apache-2.0 +// +// Object-file parsing tests for the static plugin-descriptor reader. +// +// The containers here are built by hand rather than by compiling fixture DSOs: +// it keeps the suite hermetic (no cross-compiler needed for PE or Mach-O) and, +// more importantly, it pins the wire format. A test that produced its blobs +// with the same helper the emitter uses would cancel out a layout bug instead +// of catching it, so the bytes below are written out field by field. + +#include "detail/descriptor_section_reader.hpp" + +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "pj_base/plugin_descriptor_section.hpp" + +namespace PJ::detail { +namespace { + +/// Little- or big-endian byte assembler for synthetic images. +class ImageWriter { + public: + explicit ImageWriter(bool big_endian = false) : big_endian_(big_endian) {} + + void u8(uint8_t value) { + bytes_.push_back(static_cast(value)); + } + void u16(uint16_t value) { + scalar(value, 2); + } + void u32(uint32_t value) { + scalar(value, 4); + } + void u64(uint64_t value) { + scalar(value, 8); + } + + /// Writes `text` NUL-padded to exactly `width` bytes. + void fixedName(std::string_view text, size_t width) { + for (size_t i = 0; i < width; ++i) { + u8(i < text.size() ? static_cast(text[i]) : 0); + } + } + + void raw(const std::vector& blob) { + bytes_.insert(bytes_.end(), blob.begin(), blob.end()); + } + + void padTo(size_t offset) { + while (bytes_.size() < offset) { + u8(0); + } + } + + /// Overwrites a previously reserved 32-bit field once its value is known. + void patchU32(size_t offset, uint32_t value) { + for (size_t i = 0; i < 4; ++i) { + const size_t index = big_endian_ ? 3 - i : i; + bytes_[offset + index] = static_cast((value >> (8 * i)) & 0xFF); + } + } + + [[nodiscard]] size_t size() const { + return bytes_.size(); + } + [[nodiscard]] const std::vector& bytes() const { + return bytes_; + } + + private: + void scalar(uint64_t value, size_t width) { + for (size_t i = 0; i < width; ++i) { + const size_t shift = 8 * (big_endian_ ? width - 1 - i : i); + u8(static_cast((value >> shift) & 0xFF)); + } + } + + std::vector bytes_; + bool big_endian_; +}; + +constexpr uint32_t kAbi = PJ_ABI_VERSION; + +/// One descriptor blob, laid out field by field, padded to a multiple of 8 the +/// way alignas(8) pads the real thing. `big_endian` must match the container +/// the blob is embedded in — the blob carries no endianness marker of its own, +/// it inherits the image's. +std::vector makeBlob( + uint32_t family, std::string_view manifest, uint32_t blob_version = kPluginDescriptorBlobVersion, + uint32_t manifest_size_override = 0, bool big_endian = false) { + ImageWriter writer(big_endian); + for (char c : kPluginDescriptorMagic) { + writer.u8(static_cast(c)); + } + const uint32_t json_bytes = static_cast(manifest.size()) + 1; + uint32_t blob_size = static_cast(sizeof(PluginDescriptorBlobHeader)) + json_bytes; + blob_size = (blob_size + 7U) & ~7U; + writer.u32(blob_version); + writer.u32(blob_size); + writer.u32(kAbi); + writer.u32(family); + writer.u32(manifest_size_override != 0 ? manifest_size_override : static_cast(manifest.size())); + writer.u32(0); // reserved + for (char c : manifest) { + writer.u8(static_cast(c)); + } + writer.padTo(blob_size); + return writer.bytes(); +} + +// --- Synthetic containers ---------------------------------------------------- + +constexpr uint32_t kElfSectionTypeProgBits = 1; +constexpr uint32_t kElfSectionTypeStrTab = 3; + +/// A minimal ELF with three sections: null, the payload section, .shstrtab. +std::vector makeElf( + std::string_view section_name, const std::vector& payload, bool is_64 = true, bool big_endian = false) { + const std::string names = std::string("\0", 1) + std::string(section_name) + std::string("\0.shstrtab\0", 11); + const size_t header_size = is_64 ? 64 : 52; + const size_t entry_size = is_64 ? 64 : 40; + + const size_t payload_offset = header_size; + const size_t names_offset = payload_offset + payload.size(); + const size_t table_offset = names_offset + names.size(); + + ImageWriter writer(big_endian); + writer.u8(0x7F); + writer.u8('E'); + writer.u8('L'); + writer.u8('F'); + writer.u8(is_64 ? 2 : 1); // EI_CLASS + writer.u8(big_endian ? 2 : 1); // EI_DATA + writer.padTo(16); // rest of e_ident + writer.u16(3); // e_type = ET_DYN + writer.u16(62); // e_machine + writer.u32(1); // e_version + if (is_64) { + writer.u64(0); // e_entry + writer.u64(0); // e_phoff + writer.u64(table_offset); // e_shoff + } else { + writer.u32(0); + writer.u32(0); + writer.u32(static_cast(table_offset)); + } + writer.u32(0); // e_flags + writer.u16(static_cast(header_size)); // e_ehsize + writer.u16(0); // e_phentsize + writer.u16(0); // e_phnum + writer.u16(static_cast(entry_size)); // e_shentsize + writer.u16(3); // e_shnum + writer.u16(2); // e_shstrndx + writer.padTo(payload_offset); + writer.raw(payload); + for (char c : names) { + writer.u8(static_cast(c)); + } + + const auto section = [&](uint32_t name_offset, uint32_t type, uint64_t offset, uint64_t size) { + writer.u32(name_offset); + writer.u32(type); + if (is_64) { + writer.u64(0); // sh_flags + writer.u64(0); // sh_addr + writer.u64(offset); // sh_offset + writer.u64(size); // sh_size + writer.u32(0); // sh_link + writer.u32(0); // sh_info + writer.u64(8); // sh_addralign + writer.u64(0); // sh_entsize + } else { + writer.u32(0); + writer.u32(0); + writer.u32(static_cast(offset)); + writer.u32(static_cast(size)); + writer.u32(0); + writer.u32(0); + writer.u32(8); + writer.u32(0); + } + }; + section(0, 0, 0, 0); // SHT_NULL + section(1, kElfSectionTypeProgBits, payload_offset, payload.size()); + section(static_cast(1 + section_name.size() + 1), kElfSectionTypeStrTab, names_offset, names.size()); + return writer.bytes(); +} + +/// A minimal PE image with one section. `raw_size` defaults to the payload size; +/// pass a larger value to model file-alignment padding past VirtualSize. +std::vector makePe( + std::string_view section_name, const std::vector& payload, uint32_t raw_size = 0) { + constexpr uint32_t kNtOffset = 0x80; + const uint32_t virtual_size = static_cast(payload.size()); + const uint32_t on_disk = raw_size != 0 ? raw_size : virtual_size; + + ImageWriter writer; + writer.u8('M'); + writer.u8('Z'); + writer.padTo(0x3C); + writer.u32(kNtOffset); + writer.padTo(kNtOffset); + writer.u8('P'); + writer.u8('E'); + writer.u8(0); + writer.u8(0); + writer.u16(0x8664); // Machine + writer.u16(1); // NumberOfSections + writer.u32(0); // TimeDateStamp + writer.u32(0); // PointerToSymbolTable + writer.u32(0); // NumberOfSymbols + writer.u16(0); // SizeOfOptionalHeader + writer.u16(0x2000); // Characteristics: DLL + + const size_t payload_offset_field = writer.size() + 20; + writer.fixedName(section_name, 8); + writer.u32(virtual_size); + writer.u32(0x1000); // VirtualAddress + writer.u32(on_disk); + writer.u32(0); // PointerToRawData — patched below + writer.u32(0); // PointerToRelocations + writer.u32(0); // PointerToLinenumbers + writer.u16(0); // NumberOfRelocations + writer.u16(0); // NumberOfLinenumbers + writer.u32(0x40000040); // Characteristics: initialised, read + + const auto payload_offset = static_cast(writer.size()); + writer.patchU32(payload_offset_field, payload_offset); + writer.raw(payload); + writer.padTo(payload_offset + on_disk); + return writer.bytes(); +} + +constexpr uint32_t kMachOCommandSegment64 = 0x19; + +/// A minimal 64-bit Mach-O with a single segment holding a single section. +std::vector makeMachO( + std::string_view segment_name, std::string_view section_name, const std::vector& payload) { + constexpr uint32_t kSegmentCommandSize = 72 + 80; + constexpr size_t kPayloadOffset = 32 + kSegmentCommandSize; + + ImageWriter writer; + writer.u32(0xFEEDFACFu); // MH_MAGIC_64 + writer.u32(0x0100000C); // cputype + writer.u32(0); // cpusubtype + writer.u32(6); // filetype = MH_DYLIB + writer.u32(1); // ncmds + writer.u32(kSegmentCommandSize); + writer.u32(0); // flags + writer.u32(0); // reserved + + writer.u32(kMachOCommandSegment64); + writer.u32(kSegmentCommandSize); + writer.fixedName(segment_name, 16); + writer.u64(0); // vmaddr + writer.u64(payload.size()); // vmsize + writer.u64(kPayloadOffset); // fileoff + writer.u64(payload.size()); // filesize + writer.u32(1); // maxprot + writer.u32(1); // initprot + writer.u32(1); // nsects + writer.u32(0); // flags + + writer.fixedName(section_name, 16); + writer.fixedName(segment_name, 16); + writer.u64(0); // addr + writer.u64(payload.size()); // size + writer.u32(static_cast(kPayloadOffset)); // offset + writer.u32(3); // align + writer.u32(0); // reloff + writer.u32(0); // nreloc + writer.u32(0); // flags + writer.u32(0); // reserved1 + writer.u32(0); // reserved2 + writer.u32(0); // reserved3 + + writer.padTo(kPayloadOffset); + writer.raw(payload); + return writer.bytes(); +} + +constexpr std::string_view kManifestA = R"({"id":"a","name":"A","version":"1.0.0"})"; +constexpr std::string_view kManifestB = R"({"id":"b","name":"B","version":"2.0.0"})"; + +std::vector concat(const std::vector& first, const std::vector& second) { + std::vector joined = first; + joined.insert(joined.end(), second.begin(), second.end()); + return joined; +} + +// --- ELF --------------------------------------------------------------------- + +TEST(DescriptorSectionReader, ReadsDescriptorFromElf64) { + const auto image = makeElf(PJ_PLUGIN_DESCRIPTOR_SECTION_NAME_ELF, makeBlob(kDescriptorFamilyDataSource, kManifestA)); + + auto descriptors = readEmbeddedDescriptors(image); + ASSERT_TRUE(descriptors.has_value()) << descriptors.error(); + ASSERT_EQ(descriptors->size(), 1U); + EXPECT_EQ(descriptors->front().family, kDescriptorFamilyDataSource); + EXPECT_EQ(descriptors->front().abi_version, kAbi); + EXPECT_EQ(descriptors->front().manifest_json, kManifestA); +} + +TEST(DescriptorSectionReader, ReadsDescriptorFromElf32) { + const auto image = + makeElf(PJ_PLUGIN_DESCRIPTOR_SECTION_NAME_ELF, makeBlob(kDescriptorFamilyToolbox, kManifestA), /*is_64=*/false); + + auto descriptors = readEmbeddedDescriptors(image); + ASSERT_TRUE(descriptors.has_value()) << descriptors.error(); + ASSERT_EQ(descriptors->size(), 1U); + EXPECT_EQ(descriptors->front().family, kDescriptorFamilyToolbox); + EXPECT_EQ(descriptors->front().manifest_json, kManifestA); +} + +/// Blob fields are written in the DSO's native byte order, so a big-endian +/// image needs both the section table and the blob read big-endian. +TEST(DescriptorSectionReader, ReadsDescriptorFromBigEndianElf) { + const auto blob = makeBlob( + kDescriptorFamilyMessageParser, kManifestB, kPluginDescriptorBlobVersion, /*manifest_size_override=*/0, + /*big_endian=*/true); + const auto image = makeElf(PJ_PLUGIN_DESCRIPTOR_SECTION_NAME_ELF, blob, /*is_64=*/true, /*big_endian=*/true); + + auto descriptors = readEmbeddedDescriptors(image); + ASSERT_TRUE(descriptors.has_value()) << descriptors.error(); + ASSERT_EQ(descriptors->size(), 1U); + EXPECT_EQ(descriptors->front().family, kDescriptorFamilyMessageParser); + EXPECT_EQ(descriptors->front().abi_version, kAbi); + EXPECT_EQ(descriptors->front().manifest_json, kManifestB); +} + +/// The linker pads between the contributions of different translation units, so +/// blobs are not adjacent. This is the case that forces a magic scan. +TEST(DescriptorSectionReader, ReadsSeveralDescriptorsSeparatedByPadding) { + auto payload = makeBlob(kDescriptorFamilyDataSource, kManifestA); + payload.resize(payload.size() + 24); // linker padding + payload = concat(payload, makeBlob(kDescriptorFamilyDialog, kManifestB)); + + const auto image = makeElf(PJ_PLUGIN_DESCRIPTOR_SECTION_NAME_ELF, payload); + auto descriptors = readEmbeddedDescriptors(image); + ASSERT_TRUE(descriptors.has_value()) << descriptors.error(); + ASSERT_EQ(descriptors->size(), 2U); + EXPECT_EQ(descriptors->at(0).family, kDescriptorFamilyDataSource); + EXPECT_EQ(descriptors->at(0).manifest_json, kManifestA); + EXPECT_EQ(descriptors->at(1).family, kDescriptorFamilyDialog); + EXPECT_EQ(descriptors->at(1).manifest_json, kManifestB); +} + +TEST(DescriptorSectionReader, ElfWithoutDescriptorSectionYieldsNoDescriptors) { + const auto image = makeElf(".rodata", makeBlob(kDescriptorFamilyDataSource, kManifestA)); + + auto descriptors = readEmbeddedDescriptors(image); + ASSERT_TRUE(descriptors.has_value()) << descriptors.error(); + EXPECT_TRUE(descriptors->empty()) << "a plugin with no descriptor section must fall back, not fail"; +} + +TEST(DescriptorSectionReader, SectionNameMustMatchExactly) { + const auto image = makeElf(".pj_manifest_extra", makeBlob(kDescriptorFamilyDataSource, kManifestA)); + + auto descriptors = readEmbeddedDescriptors(image); + ASSERT_TRUE(descriptors.has_value()) << descriptors.error(); + EXPECT_TRUE(descriptors->empty()) << "a longer name sharing our prefix must not match"; +} + +// --- PE ---------------------------------------------------------------------- + +TEST(DescriptorSectionReader, ReadsDescriptorFromPe) { + const auto image = makePe(PJ_PLUGIN_DESCRIPTOR_SECTION_NAME_PE, makeBlob(kDescriptorFamilyMessageParser, kManifestB)); + + auto descriptors = readEmbeddedDescriptors(image); + ASSERT_TRUE(descriptors.has_value()) << descriptors.error(); + ASSERT_EQ(descriptors->size(), 1U); + EXPECT_EQ(descriptors->front().family, kDescriptorFamilyMessageParser); + EXPECT_EQ(descriptors->front().manifest_json, kManifestB); +} + +/// SizeOfRawData is rounded up to the file alignment; the extra bytes are not +/// part of the section and must not be walked. +TEST(DescriptorSectionReader, PeIgnoresFileAlignmentPadding) { + const auto blob = makeBlob(kDescriptorFamilyToolbox, kManifestA); + const auto image = makePe(PJ_PLUGIN_DESCRIPTOR_SECTION_NAME_PE, blob, static_cast(blob.size()) + 512); + + auto descriptors = readEmbeddedDescriptors(image); + ASSERT_TRUE(descriptors.has_value()) << descriptors.error(); + ASSERT_EQ(descriptors->size(), 1U); + EXPECT_EQ(descriptors->front().manifest_json, kManifestA); +} + +TEST(DescriptorSectionReader, PeWithoutDescriptorSectionYieldsNoDescriptors) { + const auto image = makePe(".text", makeBlob(kDescriptorFamilyDataSource, kManifestA)); + + auto descriptors = readEmbeddedDescriptors(image); + ASSERT_TRUE(descriptors.has_value()) << descriptors.error(); + EXPECT_TRUE(descriptors->empty()); +} + +// --- Mach-O ------------------------------------------------------------------ + +TEST(DescriptorSectionReader, ReadsDescriptorFromMachO) { + const auto image = makeMachO("__PJ", "__manifest", makeBlob(kDescriptorFamilyDialog, kManifestB)); + + auto descriptors = readEmbeddedDescriptors(image); + ASSERT_TRUE(descriptors.has_value()) << descriptors.error(); + ASSERT_EQ(descriptors->size(), 1U); + EXPECT_EQ(descriptors->front().family, kDescriptorFamilyDialog); + EXPECT_EQ(descriptors->front().manifest_json, kManifestB); +} + +TEST(DescriptorSectionReader, MachOSegmentNameMustMatch) { + const auto image = makeMachO("__DATA", "__manifest", makeBlob(kDescriptorFamilyDialog, kManifestB)); + + auto descriptors = readEmbeddedDescriptors(image); + ASSERT_TRUE(descriptors.has_value()) << descriptors.error(); + EXPECT_TRUE(descriptors->empty()) << "the right section name in the wrong segment is not our section"; +} + +// --- Malformed input --------------------------------------------------------- + +TEST(DescriptorSectionReader, UnknownContainerIsAnError) { + const std::vector image(64, std::byte{0x5A}); + + auto descriptors = readEmbeddedDescriptors(image); + EXPECT_FALSE(descriptors.has_value()); +} + +TEST(DescriptorSectionReader, EmptyImageIsAnError) { + auto descriptors = readEmbeddedDescriptors(std::span{}); + EXPECT_FALSE(descriptors.has_value()); +} + +TEST(DescriptorSectionReader, TruncatedElfIsAnError) { + auto image = makeElf(PJ_PLUGIN_DESCRIPTOR_SECTION_NAME_ELF, makeBlob(kDescriptorFamilyDataSource, kManifestA)); + image.resize(40); // cut inside the ELF header + + auto descriptors = readEmbeddedDescriptors(image); + EXPECT_FALSE(descriptors.has_value()); +} + +TEST(DescriptorSectionReader, BlobFromAFutureLayoutIsSkipped) { + const auto image = makeElf( + PJ_PLUGIN_DESCRIPTOR_SECTION_NAME_ELF, + makeBlob(kDescriptorFamilyDataSource, kManifestA, kPluginDescriptorBlobVersion + 1)); + + auto descriptors = readEmbeddedDescriptors(image); + ASSERT_TRUE(descriptors.has_value()) << descriptors.error(); + EXPECT_TRUE(descriptors->empty()) << "an unknown blob version must be skipped, not guessed at"; +} + +TEST(DescriptorSectionReader, ManifestRunningPastTheSectionIsSkipped) { + const auto image = makeElf( + PJ_PLUGIN_DESCRIPTOR_SECTION_NAME_ELF, + makeBlob(kDescriptorFamilyDataSource, kManifestA, kPluginDescriptorBlobVersion, /*manifest_size_override=*/9999)); + + auto descriptors = readEmbeddedDescriptors(image); + ASSERT_TRUE(descriptors.has_value()) << descriptors.error(); + EXPECT_TRUE(descriptors->empty()); +} + +// --- File overload ----------------------------------------------------------- + +TEST(DescriptorSectionReader, FileAndMemoryOverloadsAgree) { + const auto image = makeElf(PJ_PLUGIN_DESCRIPTOR_SECTION_NAME_ELF, makeBlob(kDescriptorFamilyDataSource, kManifestA)); + + const auto path = + std::filesystem::temp_directory_path() / + ("pj_descriptor_reader_" + + std::to_string(static_cast(std::chrono::steady_clock::now().time_since_epoch().count())) + ".bin"); + { + std::ofstream out(path, std::ios::binary); + out.write(reinterpret_cast(image.data()), static_cast(image.size())); + } + + auto from_file = readEmbeddedDescriptors(path); + std::error_code ec; + std::filesystem::remove(path, ec); + + ASSERT_TRUE(from_file.has_value()) << from_file.error(); + ASSERT_EQ(from_file->size(), 1U); + EXPECT_EQ(from_file->front().manifest_json, kManifestA); +} + +TEST(DescriptorSectionReader, MissingFileIsAnError) { + auto descriptors = readEmbeddedDescriptors(std::filesystem::path("/nonexistent/plugin.so")); + EXPECT_FALSE(descriptors.has_value()); +} + +} // namespace +} // namespace PJ::detail diff --git a/pj_plugins/tests/plugin_catalog_test.cpp b/pj_plugins/tests/plugin_catalog_test.cpp index 9b9a1019..c4ec9149 100644 --- a/pj_plugins/tests/plugin_catalog_test.cpp +++ b/pj_plugins/tests/plugin_catalog_test.cpp @@ -170,6 +170,45 @@ TEST_F(PluginCatalogTest, ResultIsSortedByPath) { EXPECT_EQ(result->plugins[1].dso_path.filename(), pluginFileName("zz_plugin")); } +#if defined(__linux__) +/// The reason the descriptor section exists. +/// +/// dlclose does not unmap a DSO that defines STB_GNU_UNIQUE symbols, so an +/// inspect-then-close pass used to leave every scanned plugin resident for the +/// life of the process. A second copy of the same plugin loaded later from +/// another path then bound to the first copy's storage. Discovery must +/// therefore not map the image at all. +TEST_F(PluginCatalogTest, InspectingADsoDoesNotLeaveItMapped) { + const auto path = copyPlugin(PJ_MOCK_DATA_SOURCE_PLUGIN_PATH, pluginFileName("residency_probe")); + + auto descriptor = inspectPluginDso(path); + ASSERT_TRUE(descriptor.has_value()) << descriptor.error(); + + std::ifstream maps("/proc/self/maps"); + ASSERT_TRUE(maps.is_open()); + const std::string needle = path.string(); + std::string line; + bool mapped = false; + while (std::getline(maps, line)) { + if (line.find(needle) != std::string::npos) { + mapped = true; + break; + } + } + EXPECT_FALSE(mapped) << "inspectPluginDso mapped " << needle + << "; discovery must read the descriptor section instead of dlopen'ing"; +} +#endif + +/// A DSO exposing several families reports the same one the dlopen probe order +/// would have picked, so switching discovery to the static path cannot silently +/// reclassify a plugin. +TEST_F(PluginCatalogTest, MultiFamilyDsoReportsTheHighestPrecedenceFamily) { + auto descriptor = inspectPluginDso(PJ_MOCK_SOURCE_WITH_DIALOG_PLUGIN_PATH); + ASSERT_TRUE(descriptor.has_value()) << descriptor.error(); + EXPECT_EQ(descriptor->family, PluginFamily::kDataSource); +} + TEST_F(PluginCatalogTest, FamilyToStringRoundTrip) { EXPECT_EQ(toString(PluginFamily::kDataSource), "data_source"); EXPECT_EQ(toString(PluginFamily::kMessageParser), "message_parser"); From 4ec983ffcd2aea1b51757c2324107e60ca9d208f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pablo=20I=C3=B1igo=20Blasco?= Date: Mon, 3 Aug 2026 11:21:05 +0200 Subject: [PATCH 2/2] refactor(pj_plugins): sharpen NODELETE rationale and expose section-only read Review-driven follow-ups on the descriptor-section change: - Comment precision on the STB_GNU_UNIQUE mechanism. RTLD_LOCAL, -fvisibility=hidden and -Wl,-Bsymbolic-functions all fail to contain unique data symbols by construction; the rationale in library_loader.hpp, PjPluginManifest.cmake, and plugin_descriptor_section.hpp now names the first-provider rule (do_lookup_unique in glibc's dl-lookup.c) explicitly and documents which category of symbols each mechanism does and does not cover. - visibility("default") on the descriptor blob is now documented as a deliberate gc-root backup for toolchains without SHF_GNU_RETAIN, not as a redundant belt-and-braces. - Expose readSectionDescriptor() as a public primitive next to inspectPluginDso(). It returns the section-derived PluginDescriptor without ever mapping the DSO, so callers that already hold a vtable-derived descriptor from a live plugin can diff the two and surface stale-blob or packaging-mixup mismatches as diagnostics. This is not a security control: a malicious DSO can lie consistently in both places. - Test coverage for the new primitive against the mock data-source plugin. --- cmake/PjPluginManifest.cmake | 15 +++++++++++ .../pj_base/plugin_descriptor_section.hpp | 27 +++++++++++++------ .../pj_plugins/host/plugin_catalog.hpp | 19 +++++++++++++ pj_plugins/src/detail/library_loader.hpp | 24 ++++++++++++++--- pj_plugins/src/plugin_catalog.cpp | 20 ++++++++++++++ pj_plugins/tests/plugin_catalog_test.cpp | 14 ++++++++++ 6 files changed, 107 insertions(+), 12 deletions(-) diff --git a/cmake/PjPluginManifest.cmake b/cmake/PjPluginManifest.cmake index 34670c57..cc235cd8 100644 --- a/cmake/PjPluginManifest.cmake +++ b/cmake/PjPluginManifest.cmake @@ -56,6 +56,21 @@ function(pj_emit_plugin_manifest TARGET) # # -Bsymbolic-functions is Linux/ELF-specific. On macOS the linker uses # two-level namespace by default (equivalent behavior), so the flag is omitted. + # + # SCOPE CAVEAT — this pair does NOT cover STB_GNU_UNIQUE data objects + # (Meyers singletons, inline variables, template statics, thread_local, and + # their `__cxa_guard_*` guards). By construction, unique symbols are a + # process-wide data lookup that glibc funnels through a namespace-scoped + # unique table (`do_lookup_unique` in glibc's `dl-lookup.c`), and + # -Bsymbolic-functions is a function-call rewrite that does not touch data + # bindings. Any vague-linkage static a first-party plugin's transitive deps + # instantiate (libstdc++'s own templates ship with default visibility, so + # `-fvisibility=hidden` on plugin source does not silence them either) still + # reaches `.dynsym` as UNIQUE. That is what causes a bundled DSO opened for + # discovery to stay resident once its unique names are entered in the + # process's unique table — see `pj_base/plugin_descriptor_section.hpp` for + # the mechanism, and the follow-up hardening (linker version scripts or + # `-fno-gnu-unique`) that closes the load-time gap this file cannot. set_target_properties(${TARGET} PROPERTIES CXX_VISIBILITY_PRESET hidden C_VISIBILITY_PRESET hidden diff --git a/pj_base/include/pj_base/plugin_descriptor_section.hpp b/pj_base/include/pj_base/plugin_descriptor_section.hpp index f94b670a..f00d3080 100644 --- a/pj_base/include/pj_base/plugin_descriptor_section.hpp +++ b/pj_base/include/pj_base/plugin_descriptor_section.hpp @@ -14,13 +14,20 @@ // The host reads a plugin's family and manifest straight out of the DSO image // on disk, so discovery never has to dlopen the plugin. That matters because // dlclose does not necessarily unmap what dlopen mapped: glibc pins a DSO -// NODELETE as soon as it defines an STB_GNU_UNIQUE symbol (any vague-linkage -// static — inline-function locals, template statics, Meyers singletons — that -// reaches .dynsym). An inspect-then-close pass therefore leaves the plugin -// resident for the life of the process. If a second copy of the same plugin is -// later loaded from a different path, that copy binds its own references to the -// first copy's storage and finds its initialisation guards already set, so any -// layout drift between the two builds corrupts the process. +// NODELETE as soon as it is the FIRST PROVIDER of a name entered into the +// namespace's process-wide unique table (`do_lookup_unique` in glibc's +// `dl-lookup.c`). "First provider" is load-bearing here: a later copy of the +// same plugin whose unique names are already present in the table does not +// itself get pinned — it binds INTO the first copy's storage instead, which is +// what makes the duplicate-mapping bug so specific to the order things load in. +// Any vague-linkage static reaching `.dynsym` is a candidate for STB_GNU_UNIQUE +// binding (inline-function locals, template statics, Meyers singletons, +// thread_local, and their `__cxa_guard_*` guards). An inspect-then-close pass +// on a bundled plugin therefore leaves that plugin resident for the life of +// the process. If a second copy of the same plugin is later loaded from a +// different path, that copy binds its own references to the first copy's +// storage and finds its initialisation guards already set, so any layout drift +// between the two builds corrupts the process. // // Every DSO built with a PJ_*_PLUGIN macro carries one of these blobs per // family it implements. The blob duplicates the manifest string that is also @@ -58,7 +65,11 @@ #define PJ_PLUGIN_DESCRIPTOR_SECTION_NAME ".pj_manifest" #define PJ_PLUGIN_DESCRIPTOR_PLACEMENT __attribute__((section(".pj_manifest"))) // `used` keeps the compiler from dropping it; `retain` (SHF_GNU_RETAIN) keeps -// the linker from dropping it under --gc-sections. +// the linker from dropping it under --gc-sections. `visibility("default")` +// might look redundant next to those two, but it is deliberate belt-and-braces: +// on toolchains too old for `retain` / SHF_GNU_RETAIN, an exported symbol is +// itself a gc-root, and default visibility makes it exported. It is the same +// job `dllexport` does on MSVC. #if defined(__has_attribute) #if __has_attribute(retain) #define PJ_PLUGIN_DESCRIPTOR_KEEP __attribute__((visibility("default"), used, retain)) diff --git a/pj_plugins/include/pj_plugins/host/plugin_catalog.hpp b/pj_plugins/include/pj_plugins/host/plugin_catalog.hpp index 3c2095dd..c4974b71 100644 --- a/pj_plugins/include/pj_plugins/host/plugin_catalog.hpp +++ b/pj_plugins/include/pj_plugins/host/plugin_catalog.hpp @@ -16,6 +16,7 @@ #include #include +#include #include #include @@ -77,6 +78,24 @@ struct PluginScanResult { /// Inspect one DSO and return its embedded plugin descriptor. [[nodiscard]] Expected inspectPluginDso(const std::filesystem::path& dso_path); +/// Reads the manifest embedded in the DSO's descriptor section without loading +/// the plugin, and returns the parsed PluginDescriptor for the winning family +/// (the same family-precedence rule inspectPluginDso applies). +/// +/// Returns nullopt when the DSO has no descriptor section — either an old +/// plugin built against a pre-section SDK, or one carrying a hand-written +/// vtable. Returns an error when the container cannot be parsed at all or the +/// section is present but unreadable, or when an ABI-version mismatch requires +/// rejecting the plugin outright. +/// +/// Intended for callers that already hold a PluginDescriptor from a live +/// plugin (via its vtable's manifest_json) and want to cross-check that +/// discovery saw the same metadata the loaded plugin reports. A mismatch is +/// diagnostic-only: it flags a stale descriptor blob or a packaging mixup +/// against honest builds. It is not a security control — a malicious DSO can +/// lie consistently in both places. +[[nodiscard]] Expected> readSectionDescriptor(const std::filesystem::path& dso_path); + /// Recursively scan a directory for platform plugin DSOs. Invalid candidates are /// reported in diagnostics while discovery continues. [[nodiscard]] Expected scanPluginDsos(const std::filesystem::path& directory); diff --git a/pj_plugins/src/detail/library_loader.hpp b/pj_plugins/src/detail/library_loader.hpp index 8b2626a4..c6a6fee4 100644 --- a/pj_plugins/src/detail/library_loader.hpp +++ b/pj_plugins/src/detail/library_loader.hpp @@ -31,9 +31,22 @@ inline Expected loadLibraryHandle(std::string_view path) { return reinterpret_cast(module); #else // RTLD_NOW — resolve all symbols now; fail-fast on missing ones. - // RTLD_LOCAL — keep plugin symbols out of the global symbol pool; each - // plugin resolves its own copies of bundled statics in - // isolation from other plugins and from the host. + // RTLD_LOCAL — keep plugin symbols out of the global symbol pool for + // *most* symbols. Two categories still leak: + // - STB_GNU_UNIQUE data (any vague-linkage static — inline variables, + // Meyers singletons, template statics — whose defining DSO exports + // it) is deliberately global by design: glibc funnels every unique + // name through a process-wide table (see `do_lookup_unique` in + // glibc's `dl-lookup.c`), which is precisely the mechanism that + // causes a second copy of the same plugin to bind into the first + // copy's storage and skip its own constructors. RTLD_LOCAL does not + // override that; the fix is to NOT dlopen for discovery — see + // `pj_base/plugin_descriptor_section.hpp` and `inspectPluginDso`. + // - Anything the build did not manage to hide before it reached + // `.dynsym`. `-fvisibility=hidden` covers first-party code but + // libstdc++'s templates ship with default visibility, so their + // instantiations reach `.dynsym` regardless; a proper cutoff needs a + // linker version script — tracked as follow-up hardening. // // Historical note: we USED to also set RTLD_DEEPBIND on glibc to force // the plugin's own symbol scope ahead of the global one (Conan OpenSSL @@ -41,13 +54,16 @@ inline Expected loadLibraryHandle(std::string_view path) { // breaks LD_PRELOAD'd malloc interposition, which makes every plugin // dlopen fail under AddressSanitizer (and similarly for jemalloc / // tcmalloc interposition in production). Plugin-local symbol isolation - // uses two build-time mechanisms (cmake/PjPluginManifest.cmake): + // uses two build-time mechanisms (cmake/PjPluginManifest.cmake), both + // best-effort against the two carve-outs above: // 1. -fvisibility=hidden: hides symbols defined in plugin source files. // 2. -Wl,-Bsymbolic-functions (Linux): function calls within the .so // resolve to the embedded static copies, bypassing PLT. This covers // deps compiled without -fvisibility=hidden (e.g. libssl.a from Conan) // whose symbols enter the .so with default visibility and whose calls // would otherwise resolve to the host's namespace first via PLT. + // It does NOT help against STB_GNU_UNIQUE data objects — the flag is + // a function-call rewrite, not a data-lookup override. // malloc/pthread/system calls are NOT defined in the plugin so they still // reach the host — ASAN malloc interposition works correctly. int flags = RTLD_NOW | RTLD_LOCAL; diff --git a/pj_plugins/src/plugin_catalog.cpp b/pj_plugins/src/plugin_catalog.cpp index 98725fc5..e9a209c0 100644 --- a/pj_plugins/src/plugin_catalog.cpp +++ b/pj_plugins/src/plugin_catalog.cpp @@ -338,6 +338,26 @@ std::string_view toString(PluginFamily family) noexcept { return "unknown"; } +Expected> readSectionDescriptor(const std::filesystem::path& dso_path) { + if (!hasDsoSuffix(dso_path)) { + return unexpected(fmt::format("not a platform plugin DSO: {}", dso_path.string())); + } + auto with_path = [&](const std::string& error) { return fmt::format("{}: {}", dso_path.string(), error); }; + + auto embedded = staticManifestCandidate(dso_path); + if (!embedded) { + return unexpected(with_path(embedded.error())); + } + if (!embedded->has_value()) { + return std::optional{}; + } + auto descriptor = decodeManifest(dso_path, (*embedded)->family, (*embedded)->manifest_json); + if (!descriptor) { + return unexpected(with_path(descriptor.error())); + } + return std::optional(*descriptor); +} + Expected inspectPluginDso(const std::filesystem::path& dso_path) { if (!hasDsoSuffix(dso_path)) { return unexpected(fmt::format("not a platform plugin DSO: {}", dso_path.string())); diff --git a/pj_plugins/tests/plugin_catalog_test.cpp b/pj_plugins/tests/plugin_catalog_test.cpp index c4ec9149..3cee5878 100644 --- a/pj_plugins/tests/plugin_catalog_test.cpp +++ b/pj_plugins/tests/plugin_catalog_test.cpp @@ -217,5 +217,19 @@ TEST_F(PluginCatalogTest, FamilyToStringRoundTrip) { EXPECT_EQ(toString(PluginFamily::kUnknown), "unknown"); } +/// The section-derived manifest primitive that callers use to cross-check +/// against a live plugin's vtable manifest after load. Returning the same +/// descriptor discovery would report keeps the two paths honest — a stale +/// blob or packaging mixup that made them diverge would surface as a +/// diagnostic on the host runtime side (out of scope for this SDK). +TEST_F(PluginCatalogTest, ReadSectionDescriptorReturnsParsedManifest) { + auto section = readSectionDescriptor(PJ_MOCK_DATA_SOURCE_PLUGIN_PATH); + ASSERT_TRUE(section.has_value()) << section.error(); + ASSERT_TRUE(section->has_value()) << "the mock DSO carries a descriptor section by construction"; + EXPECT_EQ((*section)->family, PluginFamily::kDataSource); + EXPECT_FALSE((*section)->id.empty()); + EXPECT_FALSE((*section)->version.empty()); +} + } // namespace } // namespace PJ