feat(pj_base,pj_plugins): read plugin manifests without loading the DSO - #163
feat(pj_base,pj_plugins): read plugin manifests without loading the DSO#163pabloinigoblasco wants to merge 2 commits into
Conversation
## 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 `<elf.h>` / `<windows.h>` /
`<mach-o/loader.h>`, 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`.
|
Cross-checked this PR against two independent investigations of the duplicate-mapping bug (one of them ran live 1. Fix the isolation comments this PR's own rationale disprovesThe empirical findings contradict two existing comments that are one directory away from the new header:
Both comments should carry the GNU-unique caveat so the next reader doesn't trust the isolation claim. Suggest doing it in this PR since the new header is now the canonical description of the mechanism and can be cross-referenced. Related precision fix in 2. Consider a load-time section-vs-vtable cross-checkDedup, seeding, and compatibility decisions now run on the section manifest, while the vtable manifest "stays authoritative once the plugin is genuinely loaded" — and nothing verifies the two agree. A build-system glitch that desyncs them (stale blob after a manifest edit, packaging mixup) would silently drive winner selection with metadata the loaded plugin doesn't actually report. Suggestion: when a winner is loaded, compare the vtable manifest's 3. Retire the PE caveat with a real-DLL CI assertionThe PR notes the PE path "has not run against a real Observation (no change needed) + a caveat for the follow-up hardeningThe ELF The flip side lands in the plugin repos later: the natural follow-up hardening ( Out of scope here but queued as follow-ups: legacy section-less DSOs still take the dlopen fallback (and an old-ABI plugin is still dlopened — and potentially pinned — just to be rejected); containment for that remainder would be subprocess-based inspection. App side: submodule bump + rebuilding the bundled plugins (the fix is inert for any DSO not rebuilt against this SDK), and the AppImage packaging whitelist currently drops the 🤖 Generated with Claude Code |
…nly 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.
|
Thanks for the careful read — pushed a follow-up commit addressing the three actionable points. Precision on the first-provider mechanism. The rationale in Scope caveat on symbol-isolation flags. Section-only read exposed.
|
feat(pj_base,pj_plugins): read plugin manifests without loading the DSO
Summary
PJ_*_PLUGINmacronow 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.(
pj_plugins/src/detail/descriptor_section_reader.{hpp,cpp}) forELF32/64 (both endiannesses), PE and thin Mach-O. Fields are read at
fixed offsets rather than through
<elf.h>/<windows.h>/<mach-o/loader.h>, so all three parsers compile and are testable onany host. Only the container headers and the section itself are read:
inspecting a 50 MB plugin costs a few KB and ~2 ms.
inspectPluginDsoprefers the static path and only falls back todlopen+ vtable probe +dlclosefor a DSO with no descriptor section.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, callingits vtable entry point, and
dlclose'ing. That leaves the pluginresident. glibc pins a library
NODELETEas soon as it defines anSTB_GNU_UNIQUEsymbol, 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
UNIQUEsymbols(one representative plugin exports 1117 of them, another 358). Even
first-party plugins built with
pj_emit_plugin_manifestand its-fvisibility=hiddenexport some, because libstdc++ declaresnamespace stdwith explicit default visibility, so__gen_vtable/__to_chars_10_implinstantiations reach.dynsymanyway.Two reproducible startup crashes, both fixed here:
--plugin-dirover a bundled pluginPluginRuntimeCatalog::collectDeduplicatedPluginsExtensionCatalogService::seedBundledPluginsNotes
the static path (
protocol_version,struct_size, required slots).The contract is not loosened: the family loaders
(
data_source_library.cppand siblings) already validate every pluginat 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.
SDK has no section and still takes the
dlopenpath — includingprebuilt third-party plugins that stay exposed until they are rebuilt
against this SDK.
PJ_DIALOG_PLUGIN's body moved intoPJ_DIALOG_PLUGIN_IMPL. Onlythe form taking a manifest emits a blob; the manifest-less form still
routes to the static-link getter under
PJ_STATIC_PLUGINS.(
pj_plugin_descriptor_*). With-DPJ_ENABLE_ABI_CHECK=ON,abidiffreports it as an addition (libabigail bit 4, warning), not an
incompatible change.
abi/baseline.abiis untouched.Testing
./build.sh --debug && ./test.sh— full suite under ASAN.ctest -R descriptor_section_reader_test— 18 cases over syntheticELF64/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 particularInspectingADsoDoesNotLeaveItMapped, which fails if anyonereintroduces a
dlopeninto discovery, andMultiFamilyDsoReportsTheHighestPrecedenceFamily.one plugin id in the bundled dir and in a
--plugin-diroverride,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.
previous SDK (they report zero
.pj_manifestsections underreadelf -S).has not run against a real
.dll.