Memory corruption when using libloading

kurtlawrence · April 3, 2020, 8:07am

Hi there,

I have been using libloading to dynamically load libraries in a plugin style use case.

I have run into memory corruption issues, which I narrowed down to BTreeMap not playing nicely across the FFI boundary. The strange thing is HashMap and Vec will work across the boundary. I can work around it by using a Vec but I would like to have some of an understanding as to why BTreeMap would fail it.

I have reproduced the crash in a bare minimum repo. To reproduce you must build the dependency crate:

cd dep
cargo build
cd ..
cargo run

Any information would be really useful to help my understanding.

trzy · April 3, 2020, 9:31am

remove Cargo.lock from repo (for example .gitignore)
Why You using unsafe and ask about unsafe error?

daboross · April 3, 2020, 10:14am

Weird!

I would expect all to work, but also, none are guaranteed to work. I haven't examined the compiled code, but as a first guess, I would blame BTreeMap for it being the least trivial data structure. Both Vec and HashMap (at least last time I checked) store their internal data in one place, and then have a big allocated array of data elements. BTreeMap, on the other hand, has a whole set of different allocated nodes, and each node stores both an array of pointers and an array of data. This is an excellent case study on it's implementation, if you're interested.

Maybe the fact that BTreeMap stores pointers and data together makes it particularly susceptible to the compiler deciding to rearrange its field?

Really though, if your goal is to get this working, I wouldn't recommend spending too much effort on this. None of these structures are guaranteed to work across different binaries, and trying to guess/debug why they fail will likely read to exploring compiler and stdlib internals, and maybe finding a workaround which breaks later.

The only things that should work here are #[repr(C)] structures. Repr rust things, including structures defined by the standard library, are subject to being arbitrarily different between different binaries. Rustc can and will optimize things differently, reorder strict fields, change podding, etc. It's undefined behavior to try this with any of these data structures, so you're rolling the dice every compile, even doing this with something as simple as Vec. Undefined behavior is undefined.

If your goal is exploring why this fails, then by all means, go ahead! I recommend using gdb to step through the outer program, and examining what a BTreeMap looks like in memory in the inner and outer binaries.

If you want to make this work, and reliably work, I'd recommend adding a dependency on abi_stable. It has some macros, along with a whole set of #[repr(C)] replacements for std types and collections. Admittedly no BTreeMap, but at least it has other types which ore guaranteed to work.

daboross · April 3, 2020, 10:20am

I would object to this: for binaries and examples like this one, having Cargo.lock is extremely useful. Having it allows anyone to exactly replicate the crates the author used to compile with. Even the cargo guide itself recommends keeping Cargo.lock included (and out of .gitignore) for binary crates.

If this were a published library, I'd agree with you. But it's a crashing binary, and for that, I would always expect Cargo.lock to be included.

See also: FAQ - The Cargo Book

Yandros · April 3, 2020, 11:53am

This is similar to intertwining free() and malloc() with Rust allocations, but worse: in the former case, you can tell Rust to use the system allocator to get some form of inter-op. But in your rust-to-rust case, I don't think there is a way to tell both libraries to use a common allocator.

So you need to drop the Rust structs in the same "module" where they were created: you cannot give ownership of a String to the cdylib, nor can you drop the structs obtained from it:

libdep.so must thus export all the usual ~~suspects~~ functions: xxx_new, xxx_insert, xxx_drop (e.g., for xxx in vec, btreemap, and hashmap), and these functions must not take an owned String, but a read-only view of it, such as a shared borrow over a String or over some string slice.
Also, ABI-wise, using #[repr(Rust)] types in extern "C" signatures is not well-defined, so you should try to always use the universal C ABI: (slim) pointers. This means that the obtained collections should be Boxed, and that you should avoid fat pointers such as &str (either use &String or &&str).

Applying both changes leads to the following compiling:

//! File `dep/src/lib.rs`

use ::std::collections::*;

#[no_mangle] pub extern "C"
fn btreemap_new ()
  -> Box<BTreeMap<String, String>>
{
   Box::new(BTreeMap::new())
}

#[no_mangle] pub extern "C"
fn btreemap_insert (
    bt: &mut BTreeMap<String, String>,
    k: &String,
    v: &String,
)
{
    bt.insert(k.clone(), v.clone());
}

#[no_mangle] pub extern "C"
fn btreemap_drop (_: Box<BTreeMap<String, String>>)
{}

#[no_mangle] pub extern "C"
fn vec_new()
  -> Box<Vec<String>>
{
   Box::new(Vec::new())
}

#[no_mangle] pub extern "C"
fn vec_push (v: &mut Vec<String>, s: &String)
{
    v.push(s.clone())
}

#[no_mangle] pub extern "C"
fn vec_drop (_: Box<Vec<String>>)
{}

and then use it from src/main.rs accordingly

Hacky / dirty way I have tested it with

//! File `src/main.rs`

use ::libloading::{Library, Symbol};
use ::std::collections::*;

macro_rules! ignore_fst {(
    $fst:tt $($rest:tt)*
) => (
    $($rest)*
)}
macro_rules! p { ($expr:expr) => (
    &$expr as *const _ as *mut ()
)}

fn main ()
{
    // dropping a shared library is a very bad idea, leaking it is much better
    let lib = ::core::mem::ManuallyDrop::new(Library::new("dep/target/debug/libdep.so").unwrap());
    let lib = &*lib;
    macro_rules! call {(
        $name:ident ( $($arg:expr),* $(,)? )
    ) => ({
        let func: Symbol<unsafe extern "C" fn( $(ignore_fst!($arg *mut ()),)* ) -> *mut ()>
            = lib.get(concat!(stringify!($name), "\0").as_bytes()).unwrap()
        ;
        func($($arg ,)*)
    })};

    unsafe {
        let v = call!(vec_new());
        call!(vec_push(v, p!("Hello, World!".to_string())));
        call!(vec_drop(v));

        let bt = call!(btreemap_new());
        call!(btreemap_insert(bt, p!("Hello".to_string()), p!("World".to_string())));
        call!(btreemap_drop(bt));
    }
}

here for laziness / convenience I am auto-generating the unsafe extern "C" fn (*mut (), *mut (), ..., *mut ()) -> ... signatures out of the number of args that appear in the macro call!. This should, of course, be done instead properly with actual types.

Conclusion: using extern "C" fn is quite cumbersome and hard to do correctly, so:

daboross · April 3, 2020, 11:59am

Ah, if this is the failure, then it makes a lot of sense that only BTreeMap fails!

As I understand it, BTreeMap is the only one of the three data structures to allocate in new. Neither Vec::new nor HashMap::new allocates anything until the first push. But BTreeMap always needs at least one root node allocated to make algorithms work efficiently, so BTreeMap::new does allocate. So returning empty Vecs and HashMaps wouldn't cause allocator conflicts, but by chance BTreeMap would.

Yandros · April 3, 2020, 12:05pm

Indeed, I have just tested with dep doing Vec::with_capacity(42) and then looping over the vec part and I do get a segfault (the looping seems necessary since a use-"after"-free may not always segfault, although it will corrupt some heap memory with high probability; and once it is corrupted enough, the segfault is doomed to happen )

kurtlawrence · April 4, 2020, 9:20am

Thanks for the detailed and well written responses! So the problem is more an allocator issue rather than an ABI mismatch?

I had (possibly incorrectly) assumed that ABIs were stable across the same compiler version; it sounds like this is not guaranteed, ABIs could be different between compilations?

I was unable to reproduce that result, but I can see that it can happen. Thanks for the investigating.

I appreciate the feedback, I will rethink how I am using the plugin system.

alice · April 4, 2020, 11:23am

As far as I know, the compiler doesn't guarantee anything about the Rust ABI at this time.

trzy · April 4, 2020, 12:11pm

I cant compiling with orginal Cargo.lock
When delete it all works

Michael-F-Bryan · April 4, 2020, 12:34pm

This problem doesn't have anything to do with allocators or #[repr(...)], instead it happens because BTreeMap uses a static singleton value to represent an empty node. When a BTreeMap is destroyed it'll free each node except those pointing to this special "empty" node.

The problem comes when a BTreeMap from library A is destroyed by library B. The drop code for library B will ignore anything pointing to library B's empty node so it'll end up trying to "free" the library A empty node, which is actually pointing to static memory and not something managed by the allocator.

github.com/rust-lang/rust

Invalid free memory while dropping

opened 07:00PM - 09 Dec 19 UTC

closed 11:38AM - 31 Dec 19 UTC

sidgilles

**-------------------** **Edit** I guess this bug will not get fixed by any …hook, only by fixing it in the library. Or, at least, i did not find a workaround myself. The problem lies in the way BTreeMap is coded. See the link. https://github.com/rust-lang/rust/blob/a605441e049f0b6d5f7715b94b8ac4662fd7fcf6/src/liballoc/collections/btree/node.rs#L130 The static reference is embedded in each binary after compilation and linking process. So, i guess, the "static EMPTY_ROOT_NODE" in main exe and in the library refers to different location. See this output below. ``` $ nm /home/sidney/Developpement/test_bug/target/debug/libclib.so | grep "EMPTY_ROOT_NODE" 000000000002e780 r _ZN5alloc11collections5btree4node15EMPTY_ROOT_NODE17h0caf86d227d3ce02E $ nm /home/sidney/Developpement/test_bug/target/debug/libclib.so | rustfilt | grep "2e780" 000000000002e780 r alloc::collections::btree::node::EMPTY_ROOT_NODE $ nm /home/sidney/Developpement/test_bug/target/debug/main | grep "EMPTY_ROOT_NODE" 0000000000039ff8 R _ZN5alloc11collections5btree4node15EMPTY_ROOT_NODE17h0caf86d227d3ce02E $ nm /home/sidney/Developpement/test_bug/target/debug/main | rustfilt | grep "39ff8" 0000000000039ff8 R alloc::collections::btree::node::EMPTY_ROOT_NODE ``` Passing a BTreeMap created in main and passing it to Foo led to a bad behavior because the static reference do not point to the same memory location while dropping. I might be wrong but it make sense to me according to the previous output. **-------------------** Hello, Basically, my program is dynamically loading a library (cdylib) and retrieve a factory object (Factory design pattern). No problem here. Then, on demand, a new object with the Foo trait is created. Again, nothing wrong with that. However, moving an object created in the main program to the Foo object instance, lead to an invalid free memory, according to valgrind, when dropping the Foo object. **Edit** : This also exist on stable build --> rustc 1.39.0 (2019-11-04) **Edit** : Trying to change ["cdylib"] to ["dylib"] lead to a strange behavior : the library is loaded via 'cargo run' but no more via a regular exec by the shell (thus also by valgrind). To let you know that i did not found an understandable workaround. ``` $ rustc --version rustc 1.41.0-nightly (59947fcae 2019-12-08) $ cargo run Finished dev [unoptimized + debuginfo] target(s) in 1.29s Running `target/debug/main` --on_plugin_load--- --foo--- dropping Foo object dropping Drv2 munmap_chunk(): invalid pointer Abandon (core dumped) $ valgrind /home/sidney/Developpement/test_bug/target/debug/main ==19143== Memcheck, a memory error detector ==19143== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al. ==19143== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info ==19143== Command: /home/sidney/Developpement/test_bug/target/debug/main ==19143== --on_plugin_load--- --foo--- dropping Foo object dropping Drv2 ==19143== Invalid free() / delete / delete[] / realloc() ==19143== at 0x483897B: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so) ==19143== by 0x4EA64F2: alloc::alloc::dealloc (alloc.rs:106) ==19143== by 0x4EA63A8: <alloc::alloc::Global as core::alloc::Alloc>::dealloc (alloc.rs:177) ==19143== by 0x4EA3785: alloc::collections::btree::node::NodeRef<alloc::collections::btree::node::marker::Owned,K,V,alloc::collections::btree::node::marker::Leaf>::deallocate_and_ascend (node.rs:504) ==19143== by 0x4EA1C5B: <alloc::collections::btree::map::IntoIter<K,V> as core::ops::drop::Drop>::drop (map.rs:1431) ==19143== by 0x4EA611D: core::ptr::real_drop_in_place (mod.rs:181) ==19143== by 0x4EA2225: core::mem::drop (mod.rs:749) ==19143== by 0x4EA1BCE: <alloc::collections::btree::map::BTreeMap<K,V> as core::ops::drop::Drop>::drop (map.rs:132) ==19143== by 0x4EA60BD: core::ptr::real_drop_in_place (mod.rs:181) ==19143== by 0x4EA615D: core::ptr::real_drop_in_place (mod.rs:181) ==19143== by 0x111D07: core::ptr::real_drop_in_place (mod.rs:181) ==19143== by 0x112ED7: core::mem::drop (mod.rs:749) ==19143== Address 0x13ef38 is 0 bytes inside data symbol "_ZN5alloc11collections5btree4node15EMPTY_ROOT_NODE17h0caf86d227d3ce02E" ==19143== drop Registry #1 ==19143== ==19143== HEAP SUMMARY: ==19143== in use at exit: 3,116 bytes in 12 blocks ==19143== total heap usage: 40 allocs, 29 frees, 6,939 bytes allocated ==19143== ==19143== LEAK SUMMARY: ==19143== definitely lost: 0 bytes in 0 blocks ==19143== indirectly lost: 0 bytes in 0 blocks ==19143== possibly lost: 0 bytes in 0 blocks ==19143== still reachable: 3,116 bytes in 12 blocks ==19143== suppressed: 0 bytes in 0 blocks ==19143== Rerun with --leak-check=full to see details of leaked memory ==19143== ==19143== For counts of detected and suppressed errors, rerun with: -v ==19143== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0) ``` Here the code: Common traits definition - 'common' crate ```rust use std::any::Any; use std::collections::BTreeMap; pub trait Plugin: Any + Send + Sync + std::fmt::Debug { fn new_subplugin_trait(&self, bmap : BTreeMap<usize, usize>) -> Box<dyn Foo>; fn on_plugin_load(&self); } pub trait Foo: Any + Send + Sync + std::fmt::Debug { fn foo(&self); } ``` Cdynlib... Rust book was around... ```rust use common::*; use std::collections::BTreeMap; //use libloading::{Library, Symbol}; #[macro_export] macro_rules! declare_plugin { ($plugin_type:ty, $constructor:path) => { #[no_mangle] pub extern "C" fn _plugin_create() -> *mut dyn $crate::Plugin { // make sure the constructor is the correct type. let constructor: fn() -> $plugin_type = $constructor; let object = constructor(); //println!("Plugin : object = {:?}", object); let boxed: Box<dyn $crate::Plugin> = Box::new(object); //println!("Plugin : boxed = {:?}", boxed); let raw_pointer = Box::into_raw(boxed); //println!("Plugin : raw_pointer = {:?}", raw_pointer); return raw_pointer; } }; } //-------- #[derive(Debug)] pub(crate) struct Drv2 { bus_driver : BTreeMap<usize, usize>, } impl Drv2{ pub fn new(bmap : BTreeMap<usize, usize>,) -> Self { Self { bus_driver : bmap, } } } impl Foo for Drv2 { fn foo(&self) { println!("--foo---"); } } impl Drop for Drv2 { // mandatory. if not, segfault occurs fn drop(&mut self) { println!("dropping Drv2"); } } // ------ #[derive(Debug)] pub(crate) struct Drv { bus_driver : BTreeMap<usize, usize>, } impl Drv{ pub fn new() -> Self { Self { bus_driver : BTreeMap::<usize, usize>::new(), } } } impl Plugin for Drv { fn new_subplugin_trait(&self, bmap : BTreeMap<usize, usize>) -> Box<dyn Foo> { return Box::new(Drv2::new(bmap)); } fn on_plugin_load(&self) { println!("--on_plugin_load---"); } } declare_plugin!(Drv, Drv::new); ``` corresponding TOML ```toml [dependencies] common = { path = "../common" } # from a path in the local filesystem [lib] crate-type = ["cdylib"] ``` And finally, main program here ```rust use libloading::{Library, Symbol}; use common::Plugin; use std::collections::BTreeMap; // This Registry class maintain alive and in scope the library import and thus -- important -- the memory space allocated for it. // Loosing scope will desallocate this memory. Uses of each memory object created via this library will lead to segmentation fault. #[derive(Debug)] struct Entry { name : String, lib : libloading::Library, factory : Box<dyn Plugin>, } #[derive(Debug)] pub struct Registry { factory_list : Vec<Entry>, } impl Registry { pub fn new() -> Self { Self { factory_list : Vec::new(), } } pub fn load_from_folder(&mut self, path_str : String) -> () { unsafe { //load and reference the library if let libloading::Result::Ok(lib) = libloading::Library::new(&path_str) { if let libloading::Result::Ok(constructor) = lib.get::<libloading::Symbol<unsafe fn() -> *mut dyn Plugin>>(b"_plugin_create") { //println!("Plugin : constructor = {:?}", constructor); let boxed_raw = constructor(); //println!("Plugin : boxed_raw = {:?}", boxed_raw); let factory = Box::<dyn Plugin>::from_raw(boxed_raw); //println!("Plugin : boxed_any_trait = {:?}", factory); factory.on_plugin_load(); self.factory_list.push( Entry { name : path_str, lib : lib, factory : factory, }); //self.factory_list.push(boxed_any_trait); } } } } } impl Drop for Registry { // mandatory. if not, segfault occurs fn drop(&mut self) { println!("drop Registry #1"); for Entry { name, lib, factory, } in self.factory_list.drain(..) { //factory.on_plugin_unload(); std::mem::drop(factory); } } } fn main() { let mut a = Registry::new(); //println!("{:#?}", a); a.load_from_folder("/home/sidney/Developpement/test_bug/target/debug/libclib.so".to_string()); //println!("{:#?}", a); let mut b = a.factory_list[0].factory.new_subplugin_trait(BTreeMap::<usize, usize>::new()); //println!("{:#?}", b); b.foo(); println!("dropping Foo object"); std::mem::drop(b); } ``` and corresponding TOML ```toml [dependencies] common = { path = "../common" } # from a path in the local filesystem libloading = "0.5.2" ``` Note : i had an issue with loadlibrary because of the created Symbol lifetime. Here is another matter. Final note : this bug does not appear on zero-sized type (replacing BTreeMap by ZST_Object {} in signatures ).

See this comment from alexcrichton:

Statics guarantee a unique address, yes, and passing something like BTreeMap across dynamic library boundaries is not supported. Almost no types in the standard library are supported to cross dynamic library boundaries.

The "solution" is for the library creating the BTreeMap to also functions for freeing and manipulating it, that way it's using the correct "empty" node.

kurtlawrence · April 4, 2020, 10:53pm

This makes a lot of sense, if the BTreeMap is initialised with an entry in library A, and then dropped in library B, no memory corruption occurs.

Thanks for the additional links as well.

system · July 3, 2020, 10:53pm

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.

Topic		Replies	Views
Why does my program crash in libloading's FreeLibrary when I use a dangling pointer instead of std::ptr::copy_nonoverlapping? help	5	307	July 8, 2024
Using Rust's advanced memory management features without invalidating a bound C library's references help	7	1666	May 17, 2020
[Solved] Crash when using HashMap to track memory allocations help	2	903	January 12, 2023
Unsafe and memory alloc help	7	693	March 19, 2020
Libloading appears to be using a stale dylib help	16	1911	November 25, 2019

Memory corruption when using libloading

Related topics