| cb4670ea28d3d1f9dc68d7143f6e5c9ef4fd4a60 |
|
18-Sep-2017 |
Christopher Ferris <cferris@google.com> |
Drop thread list lock before abort.
Bug: 65656273
Test: Ran the app and verified it crashes instead of deadlocks.
Change-Id: I7dbe653d50a635a23993c99c5f73ca094ee80b28
(cherry picked from commit 7982914faefdcc6b0fbb98d63bb9fde48d4efd18)
|
| 7982914faefdcc6b0fbb98d63bb9fde48d4efd18 |
|
18-Sep-2017 |
Christopher Ferris <cferris@google.com> |
Drop thread list lock before abort.
Bug: 65656273
Test: Ran the app and verified it crashes instead of deadlocks.
Change-Id: I7dbe653d50a635a23993c99c5f73ca094ee80b28
|
| 7a3681e5b6c39bc2b3b62031ca5941dbf7bc4e63 |
|
25-Apr-2017 |
Christopher Ferris <cferris@google.com> |
Move libc_log code into libasync_safe.
This library is used by a number of different libraries in the system.
Make it easy for platform libraries to use this library and create
an actual exported include file.
Change the names of the functions to reflect the new name of the library.
Run clang_format on the async_safe_log.cpp file since the formatting is
all over the place.
Bug: 31919199
Test: Compiled for angler/bullhead, and booted.
Test: Ran bionic unit tests.
Test: Ran the malloc debug tests.
Change-Id: I8071bf690c17b0ea3bc8dc5749cdd5b6ad58478a
|
| 5450f86b31287173278513eb8b4c8e73b9fdd3ae |
|
08-Mar-2017 |
Josh Gao <jmgao@google.com> |
Fix leak of bionic TLS when threads are detached.
__pthread_internal_free doesn't happen on threads that are detached,
causing the bionic TLS allocation (and guard pages) to be leaked.
Fix the leak, and name the allocations to make things apparent if this
ever happens again.
Bug: http://b/36045112
Test: manually ran a program that detached empty threads
Change-Id: Id1c7852b7384474244f7bf5a0f7da54ff962e0a1
|
| 5e2285d3ccdbb64a49ad2e5e521f50c897a3954d |
|
22-Feb-2017 |
Josh Gao <jmgao@google.com> |
Allocate thread local buffers in __init_tls.
Thread local buffers were using pthread_setspecific for storage with
lazy initialization. pthread_setspecific shares TLS slots between the
linker and libc.so, so thread local buffers being initialized in a
different order between libc.so and the linker meant that bad things
would happen (manifesting as snprintf not working because the
locale was mangled)
Bug: http://b/20464031
Test: /data/nativetest64/bionic-unit-tests/bionic-unit-tests
everything passes
Test: /data/nativetest/bionic-unit-tests/bionic-unit-tests
thread_local tests are failing both before and after (KUSER_HELPERS?)
Test: /data/nativetest64/bionic-unit-tests-static/bionic-unit-tests-static
no additional failures
Change-Id: I9f445a77c6e86979f3fa49c4a5feecf6ec2b0c3f
|
| 6ce686c48bf6ec07a1e25b94bba7e9a3a6df4b49 |
|
21-Feb-2017 |
Elliott Hughes <enh@google.com> |
Downgrade the special case of pthread_t(0) to a warning.
So far this is the only issue we've hit in vendor code, and we've hit
it several times already. Rather than try to fix bullhead (the current
problem), let's just admit that the special case of 0 is a lot less
worrying.
Also fix the test expectations to correspond to the new abort message.
Bug: http://b/35455349 (crashes on 0)
Bug: http://b/35622944 (tests)
Test: ran tests
Change-Id: Iec57011fa699a954ebeaec151db2193e36d1ef35
|
| 3fed6ff847d0fb7409a2af4b00b6e47e1ad357ff |
|
17-Feb-2017 |
Elliott Hughes <enh@google.com> |
Include the pthread_t in the "bad pthread_t" fatal abort.
Also reword the message to be stronger.
Bug: http://b/35455349
Test: manual
Change-Id: I8f34fd42f3b635c95a7b921645a016fb303ce3ad
|
| 11859d467c035081f8acafeaf0184b670cd41c48 |
|
14-Feb-2017 |
Elliott Hughes <enh@google.com> |
Be more strict about using invalid `pthread_t`s.
Another release, another attempt to remove the global thread list.
But this time, let's admit that it's not going away. We can switch to using
a read/write lock for the global thread list, and to aborting rather than
quietly returning ESRCH if we're given an invalid pthread_t.
This change affects pthread_detach, pthread_getcpuclockid,
pthread_getschedparam/pthread_setschedparam, pthread_join, and pthread_kill:
instead of returning ESRCH when passed an invalid pthread_t, if you're
targeting O or above, they'll abort with the message "attempt to use
invalid pthread_t".
Note that this doesn't change behavior as much as you might think: the old
lookup only held the global thread list lock for the duration of the lookup,
so there was still a race between that and the dereference in the caller,
given that callers actually need the tid to pass to some syscall or other,
and sometimes update fields in the pthread_internal_t struct too.
(This patch replaces such users with calls to pthread_gettid_np, which
at least makes the TOCTOU window smaller.)
We can't check thread->tid against 0 to see whether a pthread_t is still
valid because a dead thread gets its thread struct unmapped along with its
stack, so the dereference isn't safe.
Taking the affected functions one by one:
* pthread_getcpuclockid and pthread_getschedparam/pthread_setschedparam
should be fine. Unsafe calls to those seem highly unlikely.
* Unsafe pthread_detach callers probably want to switch to
pthread_attr_setdetachstate instead, or using
pthread_detach(pthread_self()) from the new thread's start routine
rather than doing the detach in the parent.
* pthread_join calls should be safe anyway, because a joinable thread
won't actually exit and unmap until it's joined. If you're joining an
unjoinable thread, the fix is to stop marking it detached. If you're
joining an already-joined thread, you need to rethink your design.
* Unsafe pthread_kill calls aren't portably fixable. (And are obviously
inherently non-portable as-is.) The best alternative on Android is to
use pthread_gettid_np at some point that you know the thread to be
alive, and then call kill/tgkill directly.
That's still not completely safe because if you're too late, the tid
may have been reused, but then your code is inherently unsafe anyway.
Bug: http://b/19636317
Test: ran tests
Change-Id: I0372c4428e8a7f1c3af5c9334f5d9c25f2c73f21
|
| 7484c21c4c352a2200d94939fabc10d1bd3f0723 |
|
02-Feb-2017 |
Elliott Hughes <enh@google.com> |
Revert "Remove the global thread list."
This reverts commit b0e8c565a622b5519e03d4416b0b5b1a5f20d7f5.
Breaks swiftshader (http:/b/34883464).
Change-Id: I7b21193ba8a78f07d7ac65e41d0fe8516940a83b
|
| b0e8c565a622b5519e03d4416b0b5b1a5f20d7f5 |
|
04-Jan-2017 |
Elliott Hughes <enh@google.com> |
Remove the global thread list.
Another release, another attempt to fix this bug.
This change affects pthread_detach, pthread_getcpuclockid,
pthread_getschedparam/pthread_setschedparam, pthread_join, and pthread_kill:
instead of returning ESRCH when passed an invalid pthread_t, they'll now SEGV.
Note that this doesn't change behavior as much as you might think: the old
lookup only held the global thread list lock for the duration of the lookup,
so there was still a race between that and the dereference in the caller,
given that callers actually need the tid to pass to some syscall or other,
and sometimes update fields in the pthread_internal_t struct too.
We can't check thread->tid against 0 to see whether a pthread_t is still
valid because a dead thread gets its thread struct unmapped along with its
stack, so the dereference isn't safe.
Taking the affected functions one by one:
* pthread_getcpuclockid and pthread_getschedparam/pthread_setschedparam
should be fine. Unsafe calls to those seem highly unlikely.
* Unsafe pthread_detach callers probably want to switch to
pthread_attr_setdetachstate instead, or using pthread_detach(pthread_self())
from the new thread's start routine rather than doing the detach in the
parent.
* pthread_join calls should be safe anyway, because a joinable thread won't
actually exit and unmap until it's joined. If you're joining an
unjoinable thread, the fix is to stop marking it detached. If you're
joining an already-joined thread, you need to rethink your design.
* Unsafe pthread_kill calls aren't portably fixable. (And are obviously
inherently non-portable as-is.) The best alternative on Android is to
use pthread_gettid_np at some point that you know the thread to be alive,
and then call kill/tgkill directly. That's still not completely safe
because if you're too late, the tid may have been reused, but then your
code is inherently unsafe anyway.
If we find too much code is still broken, we can come back and disable
the global thread list lookups for anything targeting >= O and then have
another go at really removing this in P...
Bug: http://b/19636317
Test: N6P boots, bionic tests pass
Change-Id: Ia92641212f509344b99ee2a9bfab5383147fcba6
|
| 4bc739a54c4ba9063e91bef06ff226dab118792d |
|
05-Feb-2016 |
Dimitry Ivanov <dimitry@google.com> |
Add check for pthread_self() when looking up a thread
Check if thread_id is in fact pthread_self before
locking on g_thread_list_lock in __pthread_internal_find.
The main reason for doing this is not performance but to allow
the linker use raise() which was not working because pthread_kill()
couldn't find pthread_self() thread because the global thread
list is initialized in libc.so and the linker's version of this
list is empty.
Bug: http://b/25867917
Change-Id: I18fe620e8cd465b30f0e1ff45fff32958f3c5c00
|
| 220b99bdc1c5f51825ac2a87062bc05fe3e0d722 |
|
26-Mar-2015 |
Yabin Cui <yabinc@google.com> |
Revert "Cause Fatal error when invalid pthread_id is detected."
Some code like in https://buganizer.corp.google.com/u/0/issues/19942911 need to change first.
This reverts commit 03324780aae9ff28c8acf52debf0ea39120e5ab8.
Change-Id: I13ff1e5b3d0672bae9cde234ffba32fbbf33d338
|
| 03324780aae9ff28c8acf52debf0ea39120e5ab8 |
|
25-Mar-2015 |
Yabin Cui <yabinc@google.com> |
Cause Fatal error when invalid pthread_id is detected.
This is a patch testing whether we can use abort() instead of
returning ESRCH for invalid pthread ids. It is an intermediate
step to remove g_thread_list/g_thread_list_lock.
Bug: 19636317
Change-Id: Idd8e4a346c7ce91e1be0c2ebcb78ce51c0d0a31d
|
| 673b15e4ee2c6d99b150aedddc0f389e29f98e1b |
|
19-Mar-2015 |
Yabin Cui <yabinc@google.com> |
Let g_thread_list_lock only protect g_thread_list.
As glibc/netbsd don't protect access to thread struct members by a global
lock, we don't want to do it either. This change reduces the
responsibility of g_thread_list_lock to only protect g_thread_list.
Bug: 19636317
Change-Id: I897890710653dac165d8fa4452c7ecf74abdbf2b
|