generic-library/bionic
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

am c2eac94a: am 8f3f0418: Merge "Prevent using static-allocated pthread keys before creation."

Browse Source 
* commit 'c2eac94a8792b621765d102f918103b58bed693e':
  Prevent using static-allocated pthread keys before creation.
This commit is contained in:
Yabin Cui 2015-04-14 21:53:45 +00:00 committed by Android Git Automerger
commit 530288f1be
2 changed files with 31 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
libc/bionic/pthread_key.cpp
View File

@ -57,8 +57,15 @@ static inline bool SeqOfKeyInUse(uintptr_t seq) {
return seq & (1 << SEQ_KEY_IN_USE_BIT); return seq & (1 << SEQ_KEY_IN_USE_BIT);
} }
#define KEY_VALID_FLAG (1 << 31)
static_assert(sizeof(pthread_key_t) == sizeof(int) && static_cast<pthread_key_t>(-1) < 0,
"pthread_key_t should be typedef to int");
static inline bool KeyInValidRange(pthread_key_t key) { static inline bool KeyInValidRange(pthread_key_t key) {
return key >= 0 && key < BIONIC_PTHREAD_KEY_COUNT; // key < 0 means bit 31 is set.
// Then key < (2^31 | BIONIC_PTHREAD_KEY_COUNT) means the index part of key < BIONIC_PTHREAD_KEY_COUNT.
return (key < (KEY_VALID_FLAG | BIONIC_PTHREAD_KEY_COUNT));
} }
// Called from pthread_exit() to remove all pthread keys. This must call the destructor of // Called from pthread_exit() to remove all pthread keys. This must call the destructor of
@ -114,7 +121,7 @@ int pthread_key_create(pthread_key_t* key, void (*key_destructor)(void*)) {
while (!SeqOfKeyInUse(seq)) { while (!SeqOfKeyInUse(seq)) {
if (atomic_compare_exchange_weak(&key_map[i].seq, &seq, seq + SEQ_INCREMENT_STEP)) { if (atomic_compare_exchange_weak(&key_map[i].seq, &seq, seq + SEQ_INCREMENT_STEP)) {
atomic_store(&key_map[i].key_destructor, reinterpret_cast<uintptr_t>(key_destructor)); atomic_store(&key_map[i].key_destructor, reinterpret_cast<uintptr_t>(key_destructor));
*key = i; *key = i | KEY_VALID_FLAG;
return 0; return 0;
} }
} }
@ -127,9 +134,10 @@ int pthread_key_create(pthread_key_t* key, void (*key_destructor)(void*)) {
// responsibility of the caller to properly dispose of the corresponding data // responsibility of the caller to properly dispose of the corresponding data
// and resources, using any means it finds suitable. // and resources, using any means it finds suitable.
int pthread_key_delete(pthread_key_t key) { int pthread_key_delete(pthread_key_t key) {
if (!KeyInValidRange(key)) { if (__predict_false(!KeyInValidRange(key))) {
return EINVAL; return EINVAL;
} }
key &= ~KEY_VALID_FLAG;
// Increase seq to invalidate values in all threads. // Increase seq to invalidate values in all threads.
uintptr_t seq = atomic_load_explicit(&key_map[key].seq, memory_order_relaxed); uintptr_t seq = atomic_load_explicit(&key_map[key].seq, memory_order_relaxed);
if (SeqOfKeyInUse(seq)) { if (SeqOfKeyInUse(seq)) {
@ -141,9 +149,10 @@ int pthread_key_delete(pthread_key_t key) {
} }
void* pthread_getspecific(pthread_key_t key) { void* pthread_getspecific(pthread_key_t key) {
if (!KeyInValidRange(key)) { if (__predict_false(!KeyInValidRange(key))) {
return NULL; return NULL;
} }
key &= ~KEY_VALID_FLAG;
uintptr_t seq = atomic_load_explicit(&key_map[key].seq, memory_order_relaxed); uintptr_t seq = atomic_load_explicit(&key_map[key].seq, memory_order_relaxed);
pthread_key_data_t* data = &(__get_thread()->key_data[key]); pthread_key_data_t* data = &(__get_thread()->key_data[key]);
// It is user's responsibility to synchornize between the creation and use of pthread keys, // It is user's responsibility to synchornize between the creation and use of pthread keys,
@ -151,16 +160,19 @@ void* pthread_getspecific(pthread_key_t key) {
if (__predict_true(SeqOfKeyInUse(seq) && data->seq == seq)) { if (__predict_true(SeqOfKeyInUse(seq) && data->seq == seq)) {
return data->data; return data->data;
} }
// We arrive here when current thread holds the seq of an deleted pthread key. So the
// data is for the deleted pthread key, and should be cleared.
data->data = NULL; data->data = NULL;
return NULL; return NULL;
} }
int pthread_setspecific(pthread_key_t key, const void* ptr) { int pthread_setspecific(pthread_key_t key, const void* ptr) {
if (!KeyInValidRange(key)) { if (__predict_false(!KeyInValidRange(key))) {
return EINVAL; return EINVAL;
} }
key &= ~KEY_VALID_FLAG;
uintptr_t seq = atomic_load_explicit(&key_map[key].seq, memory_order_relaxed); uintptr_t seq = atomic_load_explicit(&key_map[key].seq, memory_order_relaxed);
if (SeqOfKeyInUse(seq)) { if (__predict_true(SeqOfKeyInUse(seq))) {
pthread_key_data_t* data = &(__get_thread()->key_data[key]); pthread_key_data_t* data = &(__get_thread()->key_data[key]);
data->seq = seq; data->seq = seq;
data->data = const_cast<void*>(ptr); data->data = const_cast<void*>(ptr);

13
tests/pthread_test.cpp
View File

@ -181,6 +181,19 @@ TEST(pthread, pthread_key_dirty) {
ASSERT_EQ(0, pthread_key_delete(key)); ASSERT_EQ(0, pthread_key_delete(key));
} }
TEST(pthread, static_pthread_key_used_before_creation) {
#if defined(__BIONIC__)
// See http://b/19625804. The bug is about a static/global pthread key being used before creation.
// So here tests if the static/global default value 0 can be detected as invalid key.
static pthread_key_t key;
ASSERT_EQ(nullptr, pthread_getspecific(key));
ASSERT_EQ(EINVAL, pthread_setspecific(key, nullptr));
ASSERT_EQ(EINVAL, pthread_key_delete(key));
#else
GTEST_LOG_(INFO) << "This test tests bionic pthread key implementation detail.\n";
#endif
}
static void* IdFn(void* arg) { static void* IdFn(void* arg) {
return arg; return arg;
} }