From 879a3e166178d16b2ffedc73899904d52285cfe8 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Wed, 22 Jun 2022 16:46:50 -0400 Subject: [PATCH] Fix: defer_rcu: futex wait: handle spurious futex wakeups MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Observed issue ============== The urcu-defer wait_defer() implements a futex wait/wakeup scheme identical to the workqueue code, which has an issue with spurious wakeups. A spurious wakeup on wait_defer can cause wait_defer to return with a defer_thread_futex state of -1, which is unexpected. It would cause the following loops in thr_defer() to decrement the defer_thread_futex to values below -1, thus actively using CPU as values will be decremented to very low negative values until it reaches 0 through underflow, or until callbacks are eventually queued. The state is restored to 0 when callbacks are found, which restores the futex state to a correct state for the following calls to wait_defer(). This issue will cause spurious unexpected high CPU use, but will not lead to data corruption. Cause ===== From futex(5): FUTEX_WAIT Returns 0 if the caller was woken up. Note that a wake-up can also be caused by common futex usage patterns in unrelated code that happened to have previously used the futex word's memory location (e.g., typical futex-based implementations of Pthreads mutexes can cause this under some conditions). Therefore, call‐ ers should always conservatively assume that a return value of 0 can mean a spurious wake-up, and use the futex word's value (i.e., the user-space synchronization scheme) to decide whether to continue to block or not. Solution ======== We therefore need to validate whether the value differs from -1 in user-space after the call to FUTEX_WAIT returns 0. Known drawbacks =============== None. Signed-off-by: Mathieu Desnoyers Change-Id: Id9c104c0bb77cc306f0b8fbf0b924bdda2aaaf4c --- src/urcu-defer-impl.h | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/src/urcu-defer-impl.h b/src/urcu-defer-impl.h index d25e9b9..b5d7926 100644 --- a/src/urcu-defer-impl.h +++ b/src/urcu-defer-impl.h @@ -194,17 +194,25 @@ static void wait_defer(void) uatomic_set(&defer_thread_futex, 0); } else { cmm_smp_rmb(); /* Read queue before read futex */ - if (uatomic_read(&defer_thread_futex) != -1) - return; - while (futex_noasync(&defer_thread_futex, FUTEX_WAIT, -1, - NULL, NULL, 0)) { + while (uatomic_read(&defer_thread_futex) == -1) { + if (!futex_noasync(&defer_thread_futex, FUTEX_WAIT, -1, NULL, NULL, 0)) { + /* + * Prior queued wakeups queued by unrelated code + * using the same address can cause futex wait to + * return 0 even through the futex value is still + * -1 (spurious wakeups). Check the value again + * in user-space to validate whether it really + * differs from -1. + */ + continue; + } switch (errno) { - case EWOULDBLOCK: + case EAGAIN: /* Value already changed. */ return; case EINTR: /* Retry if interrupted by signal. */ - break; /* Get out of switch. */ + break; /* Get out of switch. Check again. */ default: /* Unexpected error. */ urcu_die(errno); -- 2.34.1