Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation

kbaladurin
Copy link
Member

Mark thread's buffers as no longer owned before calling Thread::OnThreadTerminate because thread can delete itself in this method.

The bug could be reproduced using following example:

=================================================================
==25874==ERROR: AddressSanitizer: heap-use-after-free on address 0x62000002ef88 at pc 0x7efcac49a62e bp 0x7efca5cd3cc0 sp 0x7efca5cd3cb8
READ of size 8 at 0x62000002ef88 thread T7
    #0 0x7efcac49a62d in EventPipeBufferList* VolatileLoad<EventPipeBufferList*>(EventPipeBufferList* const*) /media/kbaladurin/data/dotnet/forked/coreclr/src/inc/volatile.h:153:13
    #1 0x7efcac48157e in DestroyThread(Thread*) /media/kbaladurin/data/dotnet/forked/coreclr/src/vm/threads.cpp:914:44
    #2 0x7efcac9aba68 in (anonymous namespace)::CreateSuspendableThread(void (*)(void*), void*, char const*)::$_0::__invoke(void*) /media/kbaladurin/data/dotnet/forked/coreclr/src/vm/gcenv.ee.cpp:1176:27
    #3 0x7efcaccd6f2a in CorUnix::CPalThread::ThreadEntry(void*) /media/kbaladurin/data/dotnet/forked/coreclr/src/pal/src/thread/thread.cpp:1684:16
    #4 0x7efcb10cc6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)
    #5 0x7efcb03593dc in clone /build/glibc-bfm8X4/glibc-2.23/misc/../sysdeps/unix/sysv/linux/x86_64/clone.S:109

0x62000002ef88 is located 3848 bytes inside of 3872-byte region [0x62000002e080,0x62000002efa0)
freed by thread T7 here:
    #0 0x509870 in operator delete(void*) (/media/kbaladurin/data/dotnet/coreclr/overlay_asan/corerun+0x509870)
    #1 0x7efcac4869f3 in Thread::DecExternalCount(int) /media/kbaladurin/data/dotnet/forked/coreclr/src/vm/threads.cpp:2561:13
    #2 0x7efcac481d22 in Thread::OnThreadTerminate(int) /media/kbaladurin/data/dotnet/forked/coreclr/src/vm/threads.cpp:3193:28
    #3 0x7efcac481576 in DestroyThread(Thread*) /media/kbaladurin/data/dotnet/forked/coreclr/src/vm/threads.cpp:908:13
    #4 0x7efcac9aba68 in (anonymous namespace)::CreateSuspendableThread(void (*)(void*), void*, char const*)::$_0::__invoke(void*) /media/kbaladurin/data/dotnet/forked/coreclr/src/vm/gcenv.ee.cpp:1176:27
    #5 0x7efcaccd6f2a in CorUnix::CPalThread::ThreadEntry(void*) /media/kbaladurin/data/dotnet/forked/coreclr/src/pal/src/thread/thread.cpp:1684:16
    #6 0x7efcb10cc6b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

This commit fixes #15436

Mark thread's buffers as no longer owned before calling
Thread::OnThreadTerminate because thread can delete itself in
this method.
@kbaladurin
Copy link
Member Author

@kbaladurin
Copy link
Member Author

@janvorli @jkotas @brianrob PTAL

Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you!

Copy link
Member

@brianrob brianrob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@brianrob brianrob merged commit ddab65b into dotnet:master Dec 8, 2017
@janvorli
Copy link
Member

janvorli commented Dec 8, 2017

@brianrob we should port this to release/2.0.0, I've just got this issue reported from other sources today.

brianrob pushed a commit to brianrob/coreclr that referenced this pull request Dec 8, 2017
Mark thread's buffers as no longer owned before calling
Thread::OnThreadTerminate because thread can delete itself in
this method.
@kbaladurin kbaladurin deleted the fix-uaf branch December 11, 2017 06:40
jashook pushed a commit to jashook/coreclr that referenced this pull request Dec 12, 2017
Mark thread's buffers as no longer owned before calling
Thread::OnThreadTerminate because thread can delete itself in
this method.
wtgodbe pushed a commit that referenced this pull request Feb 12, 2018
Mark thread's buffers as no longer owned before calling
Thread::OnThreadTerminate because thread can delete itself in
this method.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EventPipe crash during DestroyThread
3 participants