Commit graph

5 commits

Author SHA1 Message Date
Elliott Hughes
5bb113cba2 Pass caller names to __pthread_internal_find for better errors.
On http://b/122082295 we had this abort:

  12-27 15:29:31.237 10222 10814 10848 F libc    : invalid pthread_t 0xb1907960 passed to libc

This wasn't super helpful. We can do better. Now you get something like
this instead:

  03-27 02:34:58.754 25329 25329 W libc    : invalid pthread_t (0) passed to pthread_join

Test: adb shell crasher
Bug: http://b/123255692
Change-Id: I1d545665a233308480cc3747ec3120e2b6de0453
2019-02-01 16:31:10 -08:00
dimitry
fa432524a6 Mark __BIONIC_WEAK_FOR_NATIVE_BRIDGE symbols
To make it easier for Native Bridge implementations
to override these symbols.

Bug: http://b/67993967
Test: make
Change-Id: I4c53e53af494bca365dd2b3305ab0ccc2b23ba44
2017-10-27 10:01:46 +02:00
Elliott Hughes
11859d467c Be more strict about using invalid pthread_ts.
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
2017-02-13 17:59:29 -08:00
Elliott Hughes
0f001b67fe Remove "private/bionic_pthread.h".
Bug: 17476168
Change-Id: Id642987e641de81e914a28daea4ffe9d11e090ed
2014-09-12 11:35:05 -07:00
Elliott Hughes
a4831cb4a3 Add pthread_gettid_np and re-expose __get_thread for LP32.
A lot of third-party code calls the private __get_thread symbol,
often as part of a backport of bionic's pthread_rwlock implementation.
Hopefully this will go away for LP64 (since you're guaranteed the
real implementation there), but there are still APIs that take a tid
and no way to convert between a pthread_t and a tid. pthread_gettid_np
is a public API for that. To aid the transition, make __get_thread
available again for LP32.

(cherry-pick of 27efc48814b8153c55cbcd0af5d9add824816e69.)

Bug: 14079438
Change-Id: I43fabc7f1918250d31d4665ffa4ca352d0dbeac1
2014-09-11 16:22:50 -07:00