Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using atexit is unsound when loaded via dlopen/LoadLibrary #94

Closed
thomcc opened this issue Oct 11, 2020 · 7 comments
Closed

Using atexit is unsound when loaded via dlopen/LoadLibrary #94

thomcc opened this issue Oct 11, 2020 · 7 comments

Comments

@thomcc
Copy link

thomcc commented Oct 11, 2020

If I write a rust library using ctor::dtor that's loaded at runtime as a dynamic library (e.g. via dlopen/LoadLibrary/libloading::Library), my ctor functions generally will be loaded. When that library is closed, if the platform supports it it will call dtors then.

If instead of calling them on unloading, if you call them at program exit (e.g. via atexit, as #[ctor::dtor] currently does on most platforms), atexit will be calling a function pointer from a library which has been unloaded.

If you're lucky, and the library has not yet been unmapped this will work. If you're unlucky you'll get a crash, and if you're very unlucky someone hostile has arranged for that memory to be reused by their code (e.g. this is very exploitable).

It's pretty hard to repro though since in practice the function might be around in memory for a while.

Unfortunately not all platforms that will run the ctors for a dynamic library support calling close when it is closed, which I imagine is why you atexit.

@thomcc
Copy link
Author

thomcc commented Oct 11, 2020

I see that a test of this case: https://github.com/mmastrac/rust-ctor/blob/master/tests/src/dylib_load.rs, and all I can say is — yeah, it can be tough to trigger.

It's a pretty common problem though, if you look up dlclose segfault you'll find plenty of people whose programs segfault on shutdown because they dlclosed a library that used atexit. This is more or less why you find people who argue that you should never close your shared libraries (myself included, although I don't think it's okay for a safe rust library to assume users will follow this).

Personally I've seen it happen on android, if it helps, although I suspect the example is just too small to trigger it. It also might have something to do with the need to symbol merge the symbols from the rust stdlib in both.

@mmastrac
Copy link
Owner

I'm trying to recall why I relied on atexit on linux-like platforms early on. It may be worth running tests on various platforms to see how well the equivalent dtor sections work.

Unloading is hard to test because so many features on OSX will actively prevent the library from even unloading with no way to work around them (ie: thread-local storage 😢 ).

@mmastrac
Copy link
Owner

Also, leaving this here for inspiration...

https://www.mulle-kybernetik.com/weblog/2019/atexit_is_broken.html

@thomcc
Copy link
Author

thomcc commented Sep 1, 2021

This is still an issue...

@mmastrac
Copy link
Owner

mmastrac commented Sep 1, 2021

This library is designed for advanced users and there are already warnings on the README suggesting that dtor is not always the right choice. If you'd like to PR improvements to the README with better or clearer information, I'll gladly merge them. I'm considering forcing all dtor/ctor uses to be unsafe in a 1.0 version to clarify that you should know what you are doing when you use this library.

#159 covers any unsoundness/security issues... caveat emptor.

@thomcc
Copy link
Author

thomcc commented Sep 1, 2021

Making it unsafe seems like the right call, yeah

@mmastrac
Copy link
Owner

mmastrac commented Sep 1, 2021

I appreciate all your soundness feedback, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants