Does this library have expectations of soundness when it comes to rust dependencies being loaded in dylibs?

[Manishearth]

Note

This finding was identified during an agentic unsafe Rust code review performed by Gemini AI, followed by human review and verification.

During an agentic audit of popular crates, I found what might be a safety issue in this crate but only triggerable via the complicated path of dynamically loading a Rust library with a plugin, and then unloading it later.

Dynamic loading is rare, and unloading a loaded library is also rare. That said, in the context of "plugins", dynamic loading is likely to be more common.

Is that the type of thing this crate cares about? I can finish polishing up the issue summary below if so.

Original issue body

The inventory crate manages global typed plugin registries backed by intrusive singly linked lists. In src/lib.rs, each Registry stores the list head pointer in an atomic variable (head: AtomicPtr<Node>):

inventory/src/lib.rs

Lines 174 to 176 in d11c279

	pub struct Registry {
	head: AtomicPtr<Node>,
	}

When plugins are submitted via ErasedNode::submit (invoked by submit! macro static constructors), Registry::submit inserts the node's static pointer into the linked list head:

inventory/src/lib.rs

Lines 235 to 263 in d11c279

	unsafe fn submit(&'static self, new: &'static Node) {
	// The WebAssembly linker uses an unreliable heuristic to determine
	// whether a module is a "command-style" linkage, for which it will
	// insert a call to `__wasm_call_ctors` at the top of every exported
	// function. It expects that the embedder will call into such modules
	// only once per instantiation. If this heuristic goes wrong, we can end
	// up having our constructors invoked multiple times, which without this
	// safeguard would lead to our registry's linked list becoming circular.
	// On non-Wasm platforms, this check is unnecessary, so we skip it.
	#[cfg(target_family = "wasm")]
	if new.initialized.swap(true, Ordering::Relaxed) {
	return;
	}
	let mut head = self.head.load(Ordering::Relaxed);
	loop {
	unsafe {
	*new.next.get() = head.as_ref();
	}
	let new_ptr = ptr::addr_of!(*new).cast_mut();
	match self
	.head
	.compare_exchange(head, new_ptr, Ordering::Release, Ordering::Relaxed)
	{
	Ok(_) => return,
	Err(prev) => head = prev,
	}
	}
	}

When plugins are submitted inside dynamically loaded shared libraries (dylib or cdylib), OS dynamic linkers execute the static constructor sections (.init_array or .CRT$XCU) upon library loading (dlopen), inserting the library's plugin nodes into the host application's global Registry. If the application subsequently unloads the shared library via dlclose (or when dropping a libloading::Library), the operating system unmaps the memory pages containing the library's data segment, including the submitted Node and its underlying plugin value. However, Registry provides no unregistration hooks or destructors (__cxa_atexit) to remove unloaded nodes from the linked list. Consequently, Registry.head retains dangling pointers to unmapped memory.

When safe public Rust code subsequently calls inventory::iter::<T>, into_iter() loads head and dereferences the unmapped memory:

inventory/src/lib.rs

Lines 331 to 338 in d11c279

	fn into_iter<T: Collect>() -> Iter<T> {
	let head = T::registry().head.load(Ordering::Acquire);
	Iter {
	// Head pointer is always null or valid &'static Node.
	node: unsafe { head.as_ref() },
	marker: PhantomData,
	}
	}

This causes an immediate Use-After-Free (dangling pointer dereference and memory corruption or segmentation fault). While dynamic library unloading inherently involves unsafe FFI boundaries, intrusive global linked lists without unregistration hooks create an unmitigated safety hazard in dynamic plugin loading architectures. Furthermore, the public trait method ErasedNode::submit (src/lib.rs:195) lacks # Safety documentation outlining required caller lifetime invariants.

Full Audit Report

Unsafe Rust Review: inventory (v0_3)

Overall Safety Assessment

The inventory crate provides distributed typed plugin registration for Rust applications. It allows individual source files across disparate crates to submit plugin instances into a unified registry without requiring a central list. Under the hood, this is achieved by emitting static items containing plugin pointers (static __INVENTORY: Node) into OS-specific static constructor linker sections (.init_array on Linux/Android/FreeBSD, .CRT$XCU on Windows, and __DATA,__mod_init_func on macOS).

From an architectural standpoint, inventory improves significantly upon arbitrary startup execution crates (such as ctor). Plugin values submitted via inventory::submit! are evaluated at compile time as const initializers for static items (value: &{ expr }), ensuring that user-provided plugin constructors run during Compile-Time Function Evaluation (CTFE) rather than before main(). Consequently, runtime panics during startup constructor invocation are structurally impossible. At runtime, the OS loader executes generated wrapper functions (__ctor) that perform a simple lock-free singly linked list insertion (Registry::submit).

Despite this clean design, the overall safety risk is Moderate to High in dynamic loading contexts. Because the crate relies on global linked lists and static constructor sections without corresponding teardown or unregistration mechanisms (__cxa_atexit), dynamically unloading shared libraries (dlclose) containing submitted plugins inevitably leaves dangling pointers in the registry. Furthermore, the codebase contains multiple unsafe operations and macro-generated blocks that lack formal // SAFETY: proofs or re-submission invariants.

Critical Findings

Dangling pointer dereference / Use-After-Free when plugins are submitted in dynamically unloaded shared libraries (dlclose) (Unsoundness) 🔴 ⚠️

• Priority: 🔴 High

• Threat Vector: ⚠️ Accidental Misuse

• Bug Type: Double Free / UAF

When plugins are submitted via inventory::submit! inside a dynamically loaded shared library (cdylib or dylib), loading the library at runtime via dlopen invokes the OS loader, which executes the library's static constructor section (.init_array or .CRT$XCU).

During constructor invocation, the generated __ctor wrapper calls Registry::submit(&__INVENTORY), inserting the library's static __INVENTORY node into the singly linked list of the global Registry (which resides in the host application or core library defining the plugin type T).

If the application subsequently unloads the shared library via dlclose, the operating system unmaps the memory pages containing libplugin.so's data segment—including __INVENTORY, its value reference, and its next pointer. However, Registry provides no unregistration hooks or destructors (__cxa_atexit) to remove unloaded nodes from the linked list.

Consequently, REGISTRY.head in the host executable retains a pointer to unmapped memory. When safe Rust code subsequently calls inventory::iter::<T>, the iterator traverses REGISTRY.head and dereferences the dangling pointer to read node.value and node.next. This results in a segmentation fault and undefined behavior (use-after-free) triggered entirely via safe public APIs.

Fishy Findings

1. Lack of re-submission and cycle protection on non-Wasm platforms 🟡 🧪

• Priority: 🟡 Low

• Threat Vector: 🧪 Contrived Setup

• Bug Type: Data Race

On WebAssembly targets, Node includes an initialized: AtomicBool field to ensure constructor idempotency:

#[cfg(target_family = "wasm")]
if new.initialized.swap(true, Ordering::Relaxed) {
    return;
}

This safeguard prevents duplicate calls to __wasm_call_ctors from corrupting the linked list. However, this check is explicitly conditionally compiled out on all non-Wasm platforms (Linux, macOS, Windows, etc.) under the assumption that standard OS dynamic linkers execute constructor sections exactly once.

If ErasedNode::submit is invoked manually by unsafe code, or if a non-standard OS loader re-executes .init_array, submitting an already-registered Node mutates *node.next.get() without synchronization while active readers may be traversing the list (data race). Furthermore, it creates a circular reference (node.next = node), causing subsequent invocations of inventory::iter::<T> to loop infinitely.

2. Silently dropped plugins in static library archives (.a / .lib) 🟡 ⚠️

• Priority: 🟡 Low

• Threat Vector: ⚠️ Accidental Misuse

• Bug Type: Silent Linker Symbol Discarding

While submit! emits #[used] on static __CTOR, this compiler attribute only preserves the symbol during compiler dead-code elimination (DCE). When Rust crates using submit! are compiled into static library archives (.a or .lib) and linked into external C/C++ or Rust binaries, standard linkers (including MSVC link.exe without /WHOLEARCHIVE or GNU ld without --whole-archive) resolve symbols strictly on demand.

If no exported symbols from the object file containing __CTOR are explicitly referenced by the main binary, the linker discards the entire object file. As a result, submitted plugins are silently omitted from .init_array without compiler warnings or build failures.

Missing Safety Comments

The codebase lacks formal // SAFETY: comments or # Safety docstrings across almost all internal unsafe boundaries. Below are the exact file:line locations in src/lib.rs requiring rigorous proof comments:

1. src/lib.rs:189 — unsafe impl Sync for Node 🟡

The justification comment on line 187 (// The value is Sync, and next is only mutated during submit...) lacks a formal // SAFETY: prefix and complete happens-before memory ordering justification.

*Proposed Comment:

// SAFETY: `Node` contains `value: &'static dyn ErasedNode` (Sync), `initialized: AtomicBool` (Sync),
// and `next: UnsafeCell<Option<&'static Node>>` (!Sync). `next` is only mutated in `Registry::submit`
// prior to the node being published to `Registry.head` via Release CAS. Once published, `next` is
// only read by `Iter` (which loads `head` with Acquire ordering). Because each node is submitted
// at most once, writes to `next` happen-before all reads, preventing data races.

2. src/lib.rs:200 — T::registry().submit(node) inside ErasedNode 🔴

*Proposed Comment:

// SAFETY: `ErasedNode::submit`'s precondition requires `*node.value` to be of concrete type `Self` (`T`).
// `T::registry()` returns the static registry declared for `T` in `collect!`. Thus, `node.value`
// matches the plugin type managed by this registry, satisfying `Registry::submit`'s precondition.

3. src/lib.rs:251 — *new.next.get() = head.as_ref() inside Registry::submit 🔴

*Proposed Comment:

// SAFETY:
// 1. `new.next.get()` returns a valid, properly aligned pointer to `new.next`. Since `new` has not
//    yet been published to `self.head`, no other thread can concurrently read or write `new.next`.
// 2. `head.as_ref()`: `self.head` is initialized to null and only updated via atomic CAS to pointers
//    derived from `&'static Node`. Therefore, if `head` is non-null, it points to a valid `'static Node`.

4. src/lib.rs:306-307 — unsafe impl Send/Sync for void_iter::Iter<T> 🔴

*Proposed Comment:

// SAFETY: `void_iter::Iter<T>` contains `Void` (an uninhabited enum), making `Iter<T>` uninhabited.
// Because values of this struct can never exist at runtime, implementing `Send` and `Sync` unconditionally
// cannot lead to data races or thread-unsafe pointer transfers across thread boundaries.

5. src/lib.rs:335 — node: unsafe { head.as_ref() } inside into_iter 🟡

The informal comment on line 334 (// Head pointer is always null or valid &'static Node.) lacks the // SAFETY: prefix and proof invariant derivation.

*Proposed Comment:

// SAFETY: `self.head` is initialized to null and only ever mutated in `Registry::submit` via CAS
// storing pointers to valid `&'static Node` instances. Thus, `head` is either null or valid `'static Node`.

6. src/lib.rs:367 — *node.next.get() and pointer cast inside Iter::next 🔴

*Proposed Comment:

// SAFETY:
// 1. `*node.next.get()`: `node` is reachable from `Registry.head`. All reachable nodes have completed
//    submission before publication, so `next` is immutable and safe to read concurrently.
// 2. `&*value_ptr`: Every node in `T::registry()` was submitted via `<T as ErasedNode>::submit`,
//    guaranteeing `*node.value` is concrete type `T`. Casting `*const dyn ErasedNode` to `*const T`
//    and dereferencing to `&'static T` is valid and properly aligned.

7. src/lib.rs:524 — ErasedNode::submit inside macro-generated __ctor 🔴

*Proposed Comment:

// SAFETY: `__INVENTORY` is constructed with `value: &{ $value }` where `$value` is of type `$ty`.
// The compiler selects `<$ty as ErasedNode>::submit`, whose precondition requires `*__INVENTORY.value`
// to be of type `$ty`. Since `__INVENTORY.value` holds `&$value`, this precondition is satisfied.

8. Missing # Safety documentation on public trait ErasedNode (src/lib.rs:193) 🟠

ErasedNode declares unsafe fn submit(&self, node: &'static Node);. While marked #[doc(hidden)], it is a public trait accessible to downstream crates. Its docstring lacks a # Safety section detailing required caller invariants.

*Proposed Doc Comment:

/// # Safety
/// Callers must guarantee that:
/// 1. `*node.value` is of concrete type `Self`.
/// 2. `node` has not been previously submitted to any registry (to prevent circular lists and data races).
/// 3. The memory backing `node` remains allocated and mapped for the program lifetime (not dynamically unloaded).

[ratmice]

Worth noting issue #1 also pertains to both dynamically opened and closed libraries.

[Manishearth]

Good to know, thanks! I don't mind this issue being closed as a dupe, but it's probably nice to keep open because it is talking about soundness specifically (and will be useful for tracking for people showing up later).