diff --git a/libc/bionic/pthread_cond.cpp b/libc/bionic/pthread_cond.cpp index adbce07f1..d36426c16 100644 --- a/libc/bionic/pthread_cond.cpp +++ b/libc/bionic/pthread_cond.cpp @@ -172,7 +172,7 @@ static int __pthread_cond_pulse(pthread_cond_internal_t* cond, int thread_count) static int __pthread_cond_timedwait(pthread_cond_internal_t* cond, pthread_mutex_t* mutex, bool use_realtime_clock, const timespec* abs_timeout_or_null) { - int result = check_timespec(abs_timeout_or_null); + int result = check_timespec(abs_timeout_or_null, true); if (result != 0) { return result; } diff --git a/libc/bionic/pthread_mutex.cpp b/libc/bionic/pthread_mutex.cpp index cad138ca3..7d8e8a820 100644 --- a/libc/bionic/pthread_mutex.cpp +++ b/libc/bionic/pthread_mutex.cpp @@ -304,7 +304,7 @@ static inline __always_inline int __pthread_normal_mutex_lock(pthread_mutex_inte if (__predict_true(__pthread_normal_mutex_trylock(mutex, shared) == 0)) { return 0; } - int result = check_timespec(abs_timeout_or_null); + int result = check_timespec(abs_timeout_or_null, true); if (result != 0) { return result; } @@ -487,7 +487,7 @@ static int __pthread_mutex_lock_with_timeout(pthread_mutex_internal_t* mutex, old_state = new_state; } - int result = check_timespec(abs_timeout_or_null); + int result = check_timespec(abs_timeout_or_null, true); if (result != 0) { return result; } diff --git a/libc/bionic/pthread_rwlock.cpp b/libc/bionic/pthread_rwlock.cpp index b1c48c8f2..a0652953d 100644 --- a/libc/bionic/pthread_rwlock.cpp +++ b/libc/bionic/pthread_rwlock.cpp @@ -298,7 +298,7 @@ static int __pthread_rwlock_timedrdlock(pthread_rwlock_internal_t* rwlock, if (result == 0 || result == EAGAIN) { return result; } - result = check_timespec(abs_timeout_or_null); + result = check_timespec(abs_timeout_or_null, true); if (result != 0) { return result; } @@ -370,7 +370,7 @@ static int __pthread_rwlock_timedwrlock(pthread_rwlock_internal_t* rwlock, if (result == 0) { return result; } - result = check_timespec(abs_timeout_or_null); + result = check_timespec(abs_timeout_or_null, true); if (result != 0) { return result; } diff --git a/libc/bionic/semaphore.cpp b/libc/bionic/semaphore.cpp index 79b5d6372..b30c0b06e 100644 --- a/libc/bionic/semaphore.cpp +++ b/libc/bionic/semaphore.cpp @@ -235,7 +235,7 @@ int sem_timedwait(sem_t* sem, const timespec* abs_timeout) { } // Check it as per POSIX. - int result = check_timespec(abs_timeout); + int result = check_timespec(abs_timeout, false); if (result != 0) { errno = result; return -1; diff --git a/libc/private/bionic_time_conversions.h b/libc/private/bionic_time_conversions.h index 294c29a79..a83484370 100644 --- a/libc/private/bionic_time_conversions.h +++ b/libc/private/bionic_time_conversions.h @@ -47,14 +47,17 @@ __LIBC_HIDDEN__ void absolute_timespec_from_timespec(timespec& abs_ts, const tim __END_DECLS -static inline int check_timespec(const timespec* ts) { - if (ts != nullptr) { - if (ts->tv_nsec < 0 || ts->tv_nsec >= NS_PER_S) { - return EINVAL; - } - if (ts->tv_sec < 0) { - return ETIMEDOUT; - } +static inline int check_timespec(const timespec* ts, bool null_allowed) { + if (null_allowed && ts == nullptr) { + return 0; + } + // glibc just segfaults if you pass a null timespec. + // That seems a lot more likely to catch bad code than returning EINVAL. + if (ts->tv_nsec < 0 || ts->tv_nsec >= NS_PER_S) { + return EINVAL; + } + if (ts->tv_sec < 0) { + return ETIMEDOUT; } return 0; } diff --git a/tests/semaphore_test.cpp b/tests/semaphore_test.cpp index b65bfb859..84343dadd 100644 --- a/tests/semaphore_test.cpp +++ b/tests/semaphore_test.cpp @@ -131,6 +131,13 @@ TEST(semaphore, sem_timedwait) { ASSERT_EQ(0, sem_destroy(&s)); } +TEST(semaphore_DeathTest, sem_timedwait_null_timeout) { + sem_t s; + ASSERT_EQ(0, sem_init(&s, 0, 0)); + + ASSERT_EXIT(sem_timedwait(&s, nullptr), testing::KilledBySignal(SIGSEGV), ""); +} + TEST(semaphore, sem_getvalue) { sem_t s; ASSERT_EQ(0, sem_init(&s, 0, 0));