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

gcc thread_local destructor cause use after free #2519

Open
sbabbi opened this Issue May 25, 2017 · 3 comments

Comments

Projects
None yet
3 participants
@sbabbi

sbabbi commented May 25, 2017

Hello,

I posted this issue on the gcc bug tracker, but I am not entirely sure if this is supposed to be a GCC bug or a mingw bug.

I see it has also been reported here, however at the moment I can not create a sourceforge account to comment on that. I apologize if this is not the proper place.


The issue here, to my best understanding, is the following.

  • At the very first access of a thread-local object in a new program, emutls_init is called via __emutls_get_address. This function allocates some memory for the storage using malloc (see emutls_alloc), and registers emutls_destroy to be executed at thread exit, using __gthread_key_create.

  • Immediately after, the TLS init function (generated by GCC) is called. This calls the C++ constructor on the storage provided at the previous point. It also uses __gthread_key_create to register the C++ destructor for execution at thread exit.

  • When a thread terminates, _pthread_cleanup_dest in winpthread is called. This function is responsible for calling all the destructors registered with pthread_key_create. Unfortunately, the execution order of such destructors is unspecified. In this case, it happens that emutls_destroy is called before the C++ destructor, which then executes on deallocated memory.


A quick workaround would be to reverse the loop in winpthread/src/thread.c:842. This will cause the destructors to be called in reverse order, and it will probably work correctly in most of the cases.
The only other solution would be to merge emutls+TLS init on GCC side, in such a way to have a single destructor callback to both invoke the C++ destructor and cleanup the memory.

@mati865

This comment has been minimized.

Contributor

mati865 commented May 25, 2017

@lhmouse

This comment has been minimized.

Contributor

lhmouse commented May 25, 2017

Yes I knew this bug a couple of years ago... but who the heck has been caring about this problem?

The emutls thing is utterly broken. I happened to report a similar issue a few hours ago (if you LoadLibrary() a DLL and the DLL creates some thread_local objects, then FreeLibrary()'ing that DLL is very likely to lead to a crash), not to mention that __emutls_get_address() aborts the process once it runs out of memory.

So here comes the rule of thumb: Don't use thread_local on GCC for MinGW targets.

@sbabbi

This comment has been minimized.

sbabbi commented May 26, 2017

Well.. pretty sad. I might post a patch to winpthread to workaround this specific issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment