kern_umtx.c: validate clockid used for _umtx_op(2) timeouts
Only allow clocks that make sense for the umtx_sleep().
While there, do not ignore errors from kern_clock_gettime(). The invalid
clock ids are filtered, but if the clock can't be read (CLOCK_TAI before
the TAI-UTC offset is known), the functions operate on uninitialized
memory.
_umtx_op.2: correct the description of the valid _clockid for _umtx_time
And remove the duplicated list from the description of the condition
variable.
umtxq_sleep(): do not wait forever for zero absolute timeouts
For msleep(), zero sbt means no timeout. If the timeout was specified as
absolute {0, 0} on MONOTONIC/UPTIME clocks, consider the call timed out
immediately, instead of sleeping until a signal is delivered.
lib/libthr: add helper to check valid clockid to use with _umtx_op(2)
pthread_cond_clockwait(3): explicitly reject unacceptable clockids
Otherwise they are rejected by kernel in do_wait(), reached through
_thr_sleep()/UMTX_OP_WAIT_UINT_PRIVATE on the user-mode path. Then,
since cond_wait_user() only leaves its loop on a wakeup, a cancellation
or ETIMEDOUT, the EINVAL from do_wait() results in a loop.
If hit, this indicates a bug in user code, but the system library
must be resilient to such misuse.
libthr cond_wait_user(): do not loop forever on clock error
pthread_mutex_timedlock(3): return EDEADLK on detected recursion attempt
for ERRORCHECK and ADAPTIVE_NP mutexes, instead of ETIMEDOUT. This is
explicitly specified by POSIX.
libthr: add pthread_mutex_clocklock(3)
libthr: add pthread_rwlock_clock{rd,wr}lock(3)
pthread_cond_timedwait.3: list pthread_cond_clockwait as .Nm
pthread_*_timed*lock.3: switch to .Fo/Fc
pthread_mutex_clocklock.3: document
pthread_rwlock_clock{rd,wr}lock(3): document
pthread_cond_timedwait.3: split STANDARDS and HISTORYDetails
Details
- Reviewers
markj aokblast ngie - Commits
- rG7689242d6bdf: libthr: add tests for pthread_mutex_clocklock(3)
rG083b44ddc07c: libthr: add tests for pthread_rwlock_clock{rd,wr}lock(3)
rGbe90f23d4a6c: libthr: libpthread/t_mutex.c tests should expect EDEADLK
rGb3a205cd689d: pthread_cond_timedwait.3: split STANDARDS and HISTORY
rG522407f54078: pthread_rwlock_clock{rd,wr}lock(3): document
rG1c08e2b915bf: pthread_mutex_clocklock.3: document
rGa9b82d6ba374: pthread_*_timed*lock.3: switch to .Fo/Fc
rG5577567bd0d0: libthr: add pthread_rwlock_clock{rd,wr}lock(3)
rG29495576ed39: pthread_cond_timedwait.3: list pthread_cond_clockwait as .Nm
rGb7c2db4c772b: libthr: add pthread_mutex_clocklock(3)
rGbf2939ea6087: pthread_mutex_timedlock(3): return EDEADLK on detected recursion attempt
rGe81badcfc010: libthr cond_wait_user(): do not loop forever on clock error
rG8331d5a2fe96: pthread_cond_clockwait(3): explicitly reject unacceptable clockids
rG62e53516f732: lib/libthr: add helper to check valid clockid to use with _umtx_op(2)
rG8c13acc68517: umtxq_sleep(): do not wait forever for zero absolute timeouts
rG524f1bf7f9fd: kern_umtx.c: validate clockid used for _umtx_op(2) timeouts
rG701ac0b23050: _umtx_op.2: correct the description of the valid _clockid for _umtx_time
Diff Detail
Diff Detail
- Repository
- rG FreeBSD src repository
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
Comment Actions
I wonder if we need the same check on Linux side (futex) for umtx_abs_timeout_init.
| lib/libthr/thread/thr_cond.c | ||
|---|---|---|
| 350 | I think this can go commit first? | |
| lib/libthr/thread/thr_private.h | ||
| 1020 | I didn't check but if it is all type we have. Can we use CLOCK_MAX and if (clockid in [0, CLOKC_MAX])? | |
| sys/kern/kern_umtx.c | ||
| 730–733 | I think we need to make this runs unconditionally, CLOCK_TAI (absolute case) is possible to returns EINVAL in kern_clock_gettime. Withou this, a cv attack to a CLOCK_TAI can return EINVAL in __umtx_op_cv_wait (as it is the first time to use kern_clock_gettime) and userspace doesn't check EINVAL in that case. | |
| 896 | LGTM over this. You can make this go first. | |
| 3660 | Can we check timeout != NULL in the callee. I think it is reuse multiple times. | |
| lib/libthr/thread/thr_cond.c | ||
|---|---|---|
| 350 | This is a separate commit in the series. | |
| lib/libthr/thread/thr_private.h | ||
| 1020 | I prefer for this list to be explicit. The precedents are that we do add more and more type of clocks, and this list must only expand after the review. | |
| sys/kern/kern_umtx.c | ||
| 730–733 | Sorry I do not follow. What should be done unconditionally? Against what condition? | |
| 896 | Again, this is a dedicated commit in the patch series. | |
| lib/libthr/thread/thr_mutex.c | ||
|---|---|---|
| 932–939 | This can be } else if {, and then one level of indentation can be removed. | |
| sys/kern/kern_umtx.c | ||
| 730–733 | Calling kern_clock_gettime(), and checking for an error. Right now we only do it in the !absolute case. The problem is that kern_clock_gettime(CLOCK_TAI) may fail with EINVAL if the TAI offset has not yet been set. Consider __umtx_op_cv_wait() and its callers in libthr. The callers use the return value to determine whether the kernel already unlocked the associated mutex. But, with this patch, if the timeout is absolute, then EINVAL may be returned after the mutex is unlocked; if the timeout is relative, then EINVAL will be returned before the mutex is unlocked. | |
| 3660 | This is missing a statement. | |
Comment Actions
Unconditionally call kern_clock_gettime().
Deindent.
Add missed return() in compat code.
Comment Actions
I ran the full test suite with this patch, I saw a couple of new failures:
===> lib/libthr/timedmutex_test:timedmutex1
Result: failed: /home/markj/sb/main/src/contrib/netbsd-tests/lib/libpthread/t_mutex.c:725: mutex_lock(&mutex, &ts_shortlived): Resource deadlock avoided
Start time: 2026-11-09T15:03:49.512766Z
End time: 2026-11-09T15:03:49.527152Z
Duration: 0.014s
Metadata:
allowed_architectures is empty
allowed_platforms is empty
description = Checks timeout on selflock
execenv is empty
execenv_jail_params is empty
has_cleanup = false
is_exclusive = false
required_configs is empty
required_disk_space = 0
required_files is empty
required_kmods is empty
required_memory = 0
required_programs is empty
required_user is empty
timeout = 300
Standard output:
Timed mutex-test 1
Before acquiring mutex
Before endeavor to reacquire timed-mutex (timeout expected)
===> lib/libthr/timedmutex_test:timedmutex2
Result: failed: /home/markj/sb/main/src/contrib/netbsd-tests/lib/libpthread/t_mutex.c:750: mutex_lock(&mutex, &ts_shortlived): Resource deadlock avoided
Start time: 2026-11-09T15:03:49.526885Z
End time: 2026-11-09T15:03:49.543527Z
Duration: 0.017s
Metadata:
allowed_architectures is empty
allowed_platforms is empty
description = Checks timeout on selflock with timedlock
execenv is empty
execenv_jail_params is empty
has_cleanup = false
is_exclusive = false
required_configs is empty
required_disk_space = 0
required_files is empty
required_kmods is empty
required_memory = 0
required_programs is empty
required_user is empty
timeout = 300
Standard output:
Timed mutex-test 2
Before acquiring mutex with timedlock
Before endeavor to reacquire timed-mutex (timeout expected)