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.

Sign up for GitHub

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

Jump to bottom

heap-use-after-free on std::thread #12985

Closed
emaxx-google opened this issue Dec 7, 2020 · 4 comments
Closed

heap-use-after-free on std::thread #12985

emaxx-google opened this issue Dec 7, 2020 · 4 comments

Comments

@emaxx-google
Copy link

emaxx-google commented Dec 7, 2020 •
edited
Loading

The following program:

#include <thread>
int main() {
  std::thread([]{}).join();
}

compiled with -fsanitize=address (Emscripten 2.0.6):

emcc main-thread.cc -pthread -fsanitize=address -s INITIAL_MEMORY=134217728 -s PROXY_TO_PTHREAD=1 -s EXIT_RUNTIME=1

results in the following errors:

Pthread 0x3d03d80 exited.
Proxied main thread 0x3d03ec0 finished with return code 0. EXIT_RUNTIME=1 set, quitting process.
Pthread 0x3d03ec0 exited.
=================================================================
==42==ERROR: AddressSanitizer: heap-use-after-free on address 0x03d03d8c at pc 0x00033f16 bp 0x0196a6b0 sp 0x0196a6bc
WRITE of size 4 at 0x03d03d8c thread T0
    #0 0x33f16 in wasm-function[1157]+0x33f16 (a.out.wasm+0x33f16)

0x03d03d8c is located 12 bytes inside of 232-byte region [0x03d03d80,0x03d03e68)
freed by thread T0 here:
    #0 0x234e4 in wasm-function[759]+0x234e4 (a.out.wasm+0x234e4)
    #1 0x80000521  (JavaScript+0x521)
    #2 0x800007bc in Object.freeThreadData a.out.js:1980:4
    #3 0x800007c7 in Object.returnWorkerToPool a.out.js:1991:11
    #4 0x800007f4 in Worker.worker.onmessage a.out.js:2036:14
    #5 0x8000080e in Worker.<anonymous> a.out.js:2062:12
    #6 0x8000013b in Worker.emit events.js:315:20
    #7 0x800000c0 in MessagePort.<anonymous> internal/worker.js:192:55

previously allocated by thread T0 here:
    #0 0x2367d in wasm-function[760]+0x2367d (a.out.wasm+0x2367d)
    #1 0x80000521  (JavaScript+0x521)
    #2 0x80001403 in _emscripten_builtin_pthread_create a.out.js:5123:25
    #3 0x108bf in wasm-function[483]+0x108bf (a.out.wasm+0x108bf)
    #4 0x10217 in wasm-function[482]+0x10217 (a.out.wasm+0x10217)
    #5 0x15029 in wasm-function[500]+0x15029 (a.out.wasm+0x15029)
    #6 0x80000521  (JavaScript+0x521)
    #7 0x800007db in Worker.worker.onmessage a.out.js:2011:5

SUMMARY: AddressSanitizer: heap-use-after-free (a.out.wasm+0x33f15) in wasm-function[1157]+0x33f15
Shadow bytes around the buggy address:
  0x007a0760: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x007a0770: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x007a0780: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x007a0790: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x007a07a0: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
=>0x007a07b0: fd[fd]fd fd fd fd fd fd fd fd fd fd fd fd fd fd
  0x007a07c0: fd fd fd fd fd fd fd fd fd fd fd fd fd fa fa fa
  0x007a07d0: fa fa fa fa fa fa fa fa 00 00 00 00 00 00 00 00
  0x007a07e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x007a07f0: 00 00 00 00 00 fa fa fa fa fa fa fa fa fa fa fa
  0x007a0800: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==42==ABORTING

=================================================================
==42==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 4 byte(s) in 1 object(s) allocated from:
    #0 0x23a29 in wasm-function[778]+0x23a29 (a.out.wasm+0x23a29)
    #1 0x19c1 in wasm-function[62]+0x19c1 (a.out.wasm+0x19c1)
    #2 0x1809 in wasm-function[61]+0x1809 (a.out.wasm+0x1809)
    #3 0x3d36 in wasm-function[128]+0x3d36 (a.out.wasm+0x3d36)
    #4 0x800000cb  (JavaScript+0xcb)
    #5 0x800000cb  (JavaScript+0xcb)
    #6 0x15a43 in wasm-function[503]+0x15a43 (a.out.wasm+0x15a43)
    #7 0x16b98 in wasm-function[543]+0x16b98 (a.out.wasm+0x16b98)

Indirect leak of 24 byte(s) in 1 object(s) allocated from:
    #0 0x23a29 in wasm-function[778]+0x23a29 (a.out.wasm+0x23a29)
    #1 0x9581 in wasm-function[311]+0x9581 (a.out.wasm+0x9581)
    #2 0x19c8 in wasm-function[62]+0x19c8 (a.out.wasm+0x19c8)
    #3 0x1809 in wasm-function[61]+0x1809 (a.out.wasm+0x1809)
    #4 0x3d36 in wasm-function[128]+0x3d36 (a.out.wasm+0x3d36)
    #5 0x800000cb  (JavaScript+0xcb)
    #6 0x800000cb  (JavaScript+0xcb)
    #7 0x15a43 in wasm-function[503]+0x15a43 (a.out.wasm+0x15a43)

SUMMARY: AddressSanitizer: 28 byte(s) leaked in 2 allocation(s).
@emaxx-google
Copy link
Author

Hello, is there any update on this issue? I'm wondering whether it's an actual use-after-free (which could maybe be related to other threading issues, like #12988, #12970, #12971 and other flaky issues that I'm observing in my real-world application) or is it just a spurious warning.

@sbc100 sbc100 mentioned this issue Jan 12, 2021
Merged
@kripken kripken mentioned this issue Jan 12, 2021
Merged
@abujalski
Copy link
Contributor

Hi, during work on #10524 I've fixed this in bb07cde

@emaxx-google
Copy link
Author

Hi, during work on #10524 I've fixed this in bb07cde

Hi @abujalski, thanks for pointing that out! Looking forward to that PR landed (AFAICS, there's already an LGTM there - cool!).

abujalski added a commit to abujalski/emscripten that referenced this issue Jan 22, 2021
@abujalski
Fixing POSIX testsuite pthread test fails
d86abbe 
This patch aims for fixing failures in pthread related POSIX test from
clone of http://posixtest.sourceforge.net/

Following problems are addressed:
1. pthread_cond_wait/2-3 test hang
  - Disabling recursive thread cancellation.
  - Allowing __timedwait_cp to be true cancellation point as _cp
    suffix suggests
2. pthread_getschedparam/1-3 test hangs sometimes:
   In pthread_barrier_wait adding check if lock is held by main thread
   and waiting on futex in small slices of time there, to check if
   there is some work to do on behalf of Worker Threads.
3. Fixes emscripten-core#12985

Signed-off-by: Adam Bujalski <[email protected]>
abujalski added a commit to abujalski/emscripten that referenced this issue Feb 5, 2021
@abujalski
Fixing thread exit and thread cancellation (emscripten-core#12985)
2cc5955 
This patch fixes thread cancellation/exit method and also unifies why
how thread structures are updated when thread is canceled or exits.

Following problems are addressed:
1. heap-use-after-free on std::thread emscripten-core#12985
   Function `cleanupThread()` from `src/library_pthread.js`
   unconditionally set `pthread.self` member of pthread structure to 0.
   Problem was that `cleanupThread()` function might be called after
   that pthread structure has been deleted. To fix this problem, setting
   `pthread.self` field is now guarded by check if thread data hasn't
   been already freed.
2. pthread_cond_wait/2-3 test hang
  - Disabling recursive thread cancellation.
  - Allowing __timedwait_cp to be true cancellation point as _cp
    suffix suggests
3. pthread_getschedparam/1-3 test hangs sometimes:
   In pthread_barrier_wait adding check if lock is held by main thread
   and waiting on futex in small slices of time there, to check if
   there is some work to do on behalf of Worker Threads.

Signed-off-by: Adam Bujalski <[email protected]>
@abujalski abujalski mentioned this issue Feb 5, 2021
Merged
abujalski added a commit to abujalski/emscripten that referenced this issue Feb 5, 2021
@abujalski
Fixing thread exit and thread cancellation (emscripten-core#12985)
32bd7f0 
This patch fixes thread cancellation/exit method and also unifies why
how thread structures are updated when thread is canceled or exits.

Following problems are addressed:
1. heap-use-after-free on std::thread emscripten-core#12985
   Function `cleanupThread()` from `src/library_pthread.js`
   unconditionally set `pthread.self` member of pthread structure to 0.
   Problem was that `cleanupThread()` function might be called after
   that pthread structure has been deleted. To fix this problem, setting
   `pthread.self` field is now guarded by check if thread data hasn't
   been already freed.
2. pthread_cond_wait/2-3 test hang
  - Disabling recursive thread cancellation.
  - Allowing __timedwait_cp to be true cancellation point as _cp
    suffix suggests
3. pthread_getschedparam/1-3 test hangs sometimes:
   In pthread_barrier_wait adding check if lock is held by main thread
   and waiting on futex in small slices of time there, to check if
   there is some work to do on behalf of Worker Threads.

Signed-off-by: Adam Bujalski <[email protected]>
abujalski added a commit to abujalski/emscripten that referenced this issue Feb 15, 2021
@abujalski
Fixing thread exit and thread cancellation (emscripten-core#12985)
e21c1d0 
This patch fixes thread cancellation/exit method and also unifies why
how thread structures are updated when thread is canceled or exits.

Following problems are addressed:
1. heap-use-after-free on std::thread emscripten-core#12985
   Function `cleanupThread()` from `src/library_pthread.js`
   unconditionally set `pthread.self` member of pthread structure to 0.
   Problem was that `cleanupThread()` function might be called after
   that pthread structure has been deleted. To fix this problem, setting
   `pthread.self` field is now guarded by check if thread data hasn't
   been already freed.
2. pthread_cond_wait/2-3 test hang
  - Disabling recursive thread cancellation.
  - Allowing __timedwait_cp to be true cancellation point as _cp
    suffix suggests
3. pthread_getschedparam/1-3 test hangs sometimes:
   In pthread_barrier_wait adding check if lock is held by main thread
   and waiting on futex in small slices of time there, to check if
   there is some work to do on behalf of Worker Threads.

Signed-off-by: Adam Bujalski <[email protected]>
abujalski added a commit to abujalski/emscripten that referenced this issue Feb 16, 2021
@abujalski
Fixing thread exit and thread cancellation (emscripten-core#12985)
610f958 
This patch fixes thread cancellation/exit method and also unifies why
how thread structures are updated when thread is canceled or exits.

Following problems are addressed:
1. heap-use-after-free on std::thread emscripten-core#12985
   Function `cleanupThread()` from `src/library_pthread.js`
   unconditionally set `pthread.self` member of pthread structure to 0.
   Problem was that `cleanupThread()` function might be called after
   that pthread structure has been deleted. To fix this problem, setting
   `pthread.self` field is now guarded by check if thread data hasn't
   been already freed.
2. pthread_cond_wait/2-3 test hang
  - Disabling recursive thread cancellation.
  - Allowing __timedwait_cp to be true cancellation point as _cp
    suffix suggests
3. pthread_getschedparam/1-3 test hangs sometimes:
   In pthread_barrier_wait adding check if lock is held by main thread
   and waiting on futex in small slices of time there, to check if
   there is some work to do on behalf of Worker Threads.

Signed-off-by: Adam Bujalski <[email protected]>
abujalski added a commit to abujalski/emscripten that referenced this issue Feb 16, 2021
@abujalski
Fixing thread exit and thread cancellation (emscripten-core#12985)
a342a1d 
This patch fixes thread cancellation/exit method and also unifies why
how thread structures are updated when thread is canceled or exits.

Following problems are addressed:
1. heap-use-after-free on std::thread emscripten-core#12985
   Function `cleanupThread()` from `src/library_pthread.js`
   unconditionally set `pthread.self` member of pthread structure to 0.
   Problem was that `cleanupThread()` function might be called after
   that pthread structure has been deleted. To fix this problem, setting
   `pthread.self` field is now guarded by check if thread data hasn't
   been already freed.
2. pthread_cond_wait/2-3 test hang
  - Disabling recursive thread cancellation.
  - Allowing __timedwait_cp to be true cancellation point as _cp
    suffix suggests
3. pthread_getschedparam/1-3 test hangs sometimes:
   In pthread_barrier_wait adding check if lock is held by main thread
   and waiting on futex in small slices of time there, to check if
   there is some work to do on behalf of Worker Threads.

Signed-off-by: Adam Bujalski <[email protected]>
sbc100 pushed a commit that referenced this issue Feb 16, 2021
@abujalski
Fixing thread exit and thread cancellation (#12985) (#10524)
3059251 
This patch fixes thread cancellation/exit method and also unifies why
how thread structures are updated when thread is canceled or exits.

Following problems are addressed:
1. heap-use-after-free on std::thread #12985
   Function `cleanupThread()` from `src/library_pthread.js`
   unconditionally set `pthread.self` member of pthread structure to 0.
   Problem was that `cleanupThread()` function might be called after
   that pthread structure has been deleted. To fix this problem, setting
   `pthread.self` field is now guarded by check if thread data hasn't
   been already freed.
2. pthread_cond_wait/2-3 test hang
  - Disabling recursive thread cancellation.
  - Allowing __timedwait_cp to be true cancellation point as _cp
    suffix suggests
3. pthread_getschedparam/1-3 test hangs sometimes:
   In pthread_barrier_wait adding check if lock is held by main thread
   and waiting on futex in small slices of time there, to check if
   there is some work to do on behalf of Worker Threads.

Signed-off-by: Adam Bujalski <[email protected]>
@emaxx-google
Copy link
Author

Thanks for the fix, @abujalski! Closing, as I confirmed that the issue isn't reproducible on tot anymore.

@emaxx-google emaxx-google mentioned this issue Apr 6, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@emaxx-google @abujalski