Skip to content

Commit d55b448

Browse files
gburdnyh
authored andcommitted
libc: validate pthread timedlock timeout before locking; fix comment
Follow-up to the merged pthread_mutex_timedlock/clocklock (commit 92e6342) addressing review feedback: - Validate abs_timeout before the uncontended try_lock() fast path. POSIX requires pthread_mutex_timedlock/clocklock to fail with EINVAL for an invalid abs_timeout (NULL or tv_nsec out of [0,1e9)) and not acquire the mutex; previously an uncontended lock succeeded before the timeout was checked. - Replace the stray 'ponytail:' marker in the comment with a plain 'Note:'.
1 parent 92e6342 commit d55b448

1 file changed

Lines changed: 11 additions & 7 deletions

File tree

libc/pthread.cc

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -455,21 +455,25 @@ int pthread_mutex_trylock(pthread_mutex_t *m)
455455
// Try to lock @m, spinning-with-backoff until the absolute deadline given by
456456
// @abs_timeout on clock @clock. OSv's lockfree mutex has no native timed lock,
457457
// so we poll try_lock() with a short sleep between attempts. Returns 0 on
458-
// success or ETIMEDOUT if the deadline passes first.
458+
// success, EINVAL for an invalid @abs_timeout, or ETIMEDOUT if the deadline
459+
// passes first.
459460
//
460-
// ponytail: poll+backoff, not a native timed acquire. Fine for the rare
461-
// contended timedlock; if it ever needs to be tight, add a timed wait to the
462-
// lockfree mutex itself.
461+
// Note: this is a poll+backoff, not a native timed acquire. That is fine for
462+
// the rare contended timedlock; if it ever needs to be tight, add a timed wait
463+
// to the lockfree mutex itself.
463464
static int mutex_timedlock_until(pthread_mutex_t *m, clockid_t clock,
464465
const struct timespec *abs_timeout)
465466
{
466-
if (from_libc(m)->try_lock()) {
467-
return 0; // uncontended fast path
468-
}
467+
// Validate the timeout up front so an invalid @abs_timeout fails with
468+
// EINVAL without acquiring the mutex, as POSIX requires, even on the
469+
// uncontended fast path.
469470
if (!abs_timeout || abs_timeout->tv_nsec < 0 ||
470471
abs_timeout->tv_nsec >= 1000000000) {
471472
return EINVAL;
472473
}
474+
if (from_libc(m)->try_lock()) {
475+
return 0; // uncontended fast path
476+
}
473477

474478
auto deadline_ns = std::chrono::seconds(abs_timeout->tv_sec) +
475479
std::chrono::nanoseconds(abs_timeout->tv_nsec);

0 commit comments

Comments
 (0)