platform_bionic/libc/bionic/pthread_getschedparam.cpp
Elliott Hughes b0e8c565a6 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
2017-01-07 14:16:46 -08:00

45 lines
1.8 KiB
C++

/*
* Copyright (C) 2008 The Android Open Source Project
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* * Redistributions of source code must retain the above copyright
* notice, this list of conditions and the following disclaimer.
* * Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in
* the documentation and/or other materials provided with the
* distribution.
*
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
* FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
* COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
* INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
* BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
* OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
* AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
* OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
* OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
* SUCH DAMAGE.
*/
#include <errno.h>
#include "private/ErrnoRestorer.h"
#include "pthread_internal.h"
int pthread_getschedparam(pthread_t t, int* policy, sched_param* param) {
ErrnoRestorer errno_restorer;
pthread_internal_t* thread = reinterpret_cast<pthread_internal_t*>(t);
int rc = sched_getparam(thread->tid, param);
if (rc == -1) {
return errno;
}
*policy = sched_getscheduler(thread->tid);
return 0;
}