From bd0acf9da9e1c390b8fcc7e235d73a6f13e0aec8 Mon Sep 17 00:00:00 2001 From: Andrei Vagin Date: Tue, 9 May 2023 17:36:41 -0700 Subject: [PATCH] systrap: add wrappers for gcc atomic functions It makes code a bit more readable. PiperOrigin-RevId: 530758808 --- pkg/sentry/platform/systrap/sysmsg/BUILD | 2 + pkg/sentry/platform/systrap/sysmsg/atomic.h | 26 ++++ .../systrap/sysmsg/sighandler_amd64.c | 21 ++-- .../systrap/sysmsg/sighandler_arm64.c | 11 +- .../platform/systrap/sysmsg/sysmsg_lib.c | 111 ++++++++---------- 5 files changed, 93 insertions(+), 78 deletions(-) create mode 100644 pkg/sentry/platform/systrap/sysmsg/atomic.h diff --git a/pkg/sentry/platform/systrap/sysmsg/BUILD b/pkg/sentry/platform/systrap/sysmsg/BUILD index 2fd68ef83..43c583b09 100644 --- a/pkg/sentry/platform/systrap/sysmsg/BUILD +++ b/pkg/sentry/platform/systrap/sysmsg/BUILD @@ -21,6 +21,7 @@ cc_pie_obj( ], arm64 = ["sighandler_arm64.c"], ) + [ + "atomic.h", "sysmsg.h", "sysmsg_offsets.h", ], @@ -30,6 +31,7 @@ cc_pie_obj( cc_pie_obj( name = "sysmsg_lib", srcs = [ + "atomic.h", "sysmsg.h", "sysmsg_lib.c", "sysmsg_offsets.h", diff --git a/pkg/sentry/platform/systrap/sysmsg/atomic.h b/pkg/sentry/platform/systrap/sysmsg/atomic.h new file mode 100644 index 000000000..c14eecba4 --- /dev/null +++ b/pkg/sentry/platform/systrap/sysmsg/atomic.h @@ -0,0 +1,26 @@ +// Copyright 2023 The gVisor Authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef THIRD_PARTY_GVISOR_PKG_SENTRY_PLATFORM_SYSTRAP_SYSMSG_ATOMIC_H_ +#define THIRD_PARTY_GVISOR_PKG_SENTRY_PLATFORM_SYSTRAP_SYSMSG_ATOMIC_H_ + +#define atomic_load(p) __atomic_load_n(p, __ATOMIC_ACQUIRE) +#define atomic_store(p, val) __atomic_store_n(p, val, __ATOMIC_RELEASE) +#define atomic_compare_exchange(p, old, val) \ + __atomic_compare_exchange_n(p, old, val, false, __ATOMIC_ACQ_REL, \ + __ATOMIC_ACQUIRE) +#define atomic_add(p, val) __atomic_add_fetch(p, val, __ATOMIC_ACQ_REL) +#define atomic_sub(p, val) __atomic_sub_fetch(p, val, __ATOMIC_ACQ_REL) + +#endif // THIRD_PARTY_GVISOR_PKG_SENTRY_PLATFORM_SYSTRAP_SYSMSG_ATOMIC_H_ diff --git a/pkg/sentry/platform/systrap/sysmsg/sighandler_amd64.c b/pkg/sentry/platform/systrap/sysmsg/sighandler_amd64.c index f138627a9..e7d7693ba 100644 --- a/pkg/sentry/platform/systrap/sysmsg/sighandler_amd64.c +++ b/pkg/sentry/platform/systrap/sysmsg/sighandler_amd64.c @@ -27,6 +27,7 @@ #include #include +#include "atomic.h" #include "sysmsg.h" #include "sysmsg_offsets.h" #include "sysmsg_offsets_amd64.h" @@ -168,13 +169,13 @@ struct thread_context *switch_context_amd64( // After setting THREAD_STATE_NONE, syshandled can be interrupted by // SIGCHLD. In this case, we consider that the current context contains // the actual state and sighandler can take control on it. - __atomic_store_n(&sysmsg->state, THREAD_STATE_NONE, __ATOMIC_RELEASE); - if (__atomic_load_n(&ctx->interrupt, __ATOMIC_ACQUIRE) != 0) { - __atomic_store_n(&sysmsg->state, THREAD_STATE_PREP, __ATOMIC_RELEASE); + atomic_store(&sysmsg->state, THREAD_STATE_NONE); + if (atomic_load(&ctx->interrupt) != 0) { + atomic_store(&sysmsg->state, THREAD_STATE_PREP); // This context got interrupted while it was waiting in the queue. // Setup all the necessary bits to let the sentry know this context has // switched back because of it. - __atomic_store_n(&ctx->interrupt, 0, __ATOMIC_RELEASE); + atomic_store(&ctx->interrupt, 0); new_context_state = CONTEXT_STATE_FAULT; ctx->signo = SIGCHLD; ctx->siginfo.si_signo = SIGCHLD; @@ -198,7 +199,7 @@ void __export_sighandler(int signo, siginfo_t *siginfo, void *_ucontext) { struct sysmsg *sysmsg = sysmsg_addr(sp); if (sysmsg != sysmsg->self) panic(0xdeaddead); - int32_t thread_state = __atomic_load_n(&sysmsg->state, __ATOMIC_ACQUIRE); + int32_t thread_state = atomic_load(&sysmsg->state); if (thread_state == THREAD_STATE_INITIALIZING) { // This thread was interrupted before it even had a context. return; @@ -233,7 +234,7 @@ void __export_sighandler(int signo, siginfo_t *siginfo, void *_ucontext) { memcpy(ctx->fpstate, (uint8_t *)ucontext->uc_mcontext.fpregs, __export_arch_state.fp_len); - __atomic_store_n(&ctx->fpstate_changed, 0, __ATOMIC_RELEASE); + atomic_store(&ctx->fpstate_changed, 0); } enum context_state ctx_state = CONTEXT_STATE_INVALID; @@ -261,12 +262,12 @@ void __export_sighandler(int signo, siginfo_t *siginfo, void *_ucontext) { // will mean that the first copy is in the consistent state. for (int i = 0; i < 2; i++) { // fault_jump is set to the size of "mov (%rbx)" which is 3 bytes. - __atomic_store_n(&sysmsg->fault_jump, 3, __ATOMIC_RELEASE); + atomic_store(&sysmsg->fault_jump, 3); asm volatile("movq (%1), %0\n" : "=a"(syscall_code_int[i]) : "b"(rip - 8) : "cc", "memory"); - __atomic_store_n(&sysmsg->fault_jump, 0, __ATOMIC_RELEASE); + atomic_store(&sysmsg->fault_jump, 0); } // The mov instruction is 5 bytes: b8 . // The syscall instruction is 2 bytes: 0f 05. @@ -324,7 +325,7 @@ void __export_sighandler(int signo, siginfo_t *siginfo, void *_ucontext) { set_fsbase(ctx->ptregs.fs_base); } - if (__atomic_load_n(&ctx->fpstate_changed, __ATOMIC_ACQUIRE)) { + if (atomic_load(&ctx->fpstate_changed)) { prep_fpstate_for_sigframe( ctx->fpstate, __export_arch_state.fp_len, __export_arch_state.xsave_mode != XSAVE_MODE_FXSAVE); @@ -338,7 +339,7 @@ void __syshandler() { asm volatile("movq %%gs:0, %0\n" : "=r"(sysmsg) : :); // SYSMSG_STATE_PREP is set to postpone interrupts. Look at // __export_sighandler for more details. - int state = __atomic_load_n(&sysmsg->state, __ATOMIC_ACQUIRE); + int state = atomic_load(&sysmsg->state); if (state != THREAD_STATE_PREP) panic(state); struct thread_context *ctx = sysmsg->context; diff --git a/pkg/sentry/platform/systrap/sysmsg/sighandler_arm64.c b/pkg/sentry/platform/systrap/sysmsg/sighandler_arm64.c index d0e04e46d..09a47a2af 100644 --- a/pkg/sentry/platform/systrap/sysmsg/sighandler_arm64.c +++ b/pkg/sentry/platform/systrap/sysmsg/sighandler_arm64.c @@ -26,6 +26,7 @@ #include #include +#include "atomic.h" #include "sysmsg.h" #include "sysmsg_offsets.h" @@ -100,7 +101,7 @@ void __export_sighandler(int signo, siginfo_t *siginfo, void *_ucontext) { struct sysmsg *sysmsg = sysmsg_addr(sp); if (sysmsg != sysmsg->self) panic(0xdeaddead); - int32_t thread_state = __atomic_load_n(&sysmsg->state, __ATOMIC_ACQUIRE); + int32_t thread_state = atomic_load(&sysmsg->state); uint32_t ctx_state = CONTEXT_STATE_INVALID; struct thread_context *ctx = NULL, *old_ctx = NULL; @@ -159,11 +160,11 @@ init: for (;;) { ctx = switch_context(sysmsg, ctx, ctx_state); - if (__atomic_load_n(&ctx->interrupt, __ATOMIC_ACQUIRE) != 0) { + if (atomic_load(&ctx->interrupt) != 0) { // This context got interrupted while it was waiting in the queue. // Setup all the necessary bits to let the sentry know this context has // switched back because of it. - __atomic_store_n(&ctx->interrupt, 0, __ATOMIC_RELEASE); + atomic_store(&ctx->interrupt, 0); ctx_state = CONTEXT_STATE_FAULT; ctx->signo = SIGCHLD; ctx->siginfo.si_signo = SIGCHLD; @@ -187,10 +188,10 @@ void restore_state(struct sysmsg *sysmsg, struct thread_context *ctx, (struct fpsimd_context *)&ucontext->uc_mcontext.__reserved; uint8_t *fpStatePointer = (uint8_t *)&fpctx->fpsr; - if (__atomic_load_n(&ctx->fpstate_changed, __ATOMIC_ACQUIRE)) { + if (atomic_load(&ctx->fpstate_changed)) { memcpy(fpStatePointer, ctx->fpstate, __export_arch_state.fp_len); } ptregs_to_gregs(ucontext, &ctx->ptregs); set_tls(ctx->tls); - __atomic_store_n(&sysmsg->state, THREAD_STATE_NONE, __ATOMIC_RELEASE); + atomic_store(&sysmsg->state, THREAD_STATE_NONE); } diff --git a/pkg/sentry/platform/systrap/sysmsg/sysmsg_lib.c b/pkg/sentry/platform/systrap/sysmsg/sysmsg_lib.c index 69c1aed9d..252ed5e97 100644 --- a/pkg/sentry/platform/systrap/sysmsg/sysmsg_lib.c +++ b/pkg/sentry/platform/systrap/sysmsg/sysmsg_lib.c @@ -22,6 +22,7 @@ #include #include +#include "atomic.h" #include "sysmsg.h" // __export_deep_sleep_timeout is the timeout after which the stub thread stops @@ -57,14 +58,12 @@ struct context_queue *__export_context_queue_addr; // LINT.ThenChange(../context_queue.go) uint32_t is_empty(struct context_queue *queue) { - return __atomic_load_n(&queue->start, __ATOMIC_ACQUIRE) == - __atomic_load_n(&queue->end, __ATOMIC_ACQUIRE); + return atomic_load(&queue->start) == atomic_load(&queue->end); } int32_t queued_contexts(struct context_queue *queue) { - return (__atomic_load_n(&queue->end, __ATOMIC_ACQUIRE) + - MAX_CONTEXT_QUEUE_ENTRIES - - __atomic_load_n(&queue->start, __ATOMIC_ACQUIRE)) % + return (atomic_load(&queue->end) + MAX_CONTEXT_QUEUE_ENTRIES - + atomic_load(&queue->start)) % MAX_CONTEXT_QUEUE_ENTRIES; } @@ -130,16 +129,15 @@ static bool spinning_queue_push(void) { BUILD_BUG_ON(sizeof(struct spinning_queue) > SPINNING_QUEUE_MEM_SIZE); - end = __atomic_add_fetch(&queue->end, 1, __ATOMIC_SEQ_CST); - start = __atomic_load_n(&queue->start, __ATOMIC_SEQ_CST); + end = atomic_add(&queue->end, 1); + start = atomic_load(&queue->start); if (end - start > MAX_SPINNING_THREADS) { - __atomic_sub_fetch(&queue->end, 1, __ATOMIC_SEQ_CST); + atomic_sub(&queue->end, 1); return false; } idx = end - 1; - __atomic_store_n(&queue->start_times[idx % SPINNING_QUEUE_SIZE], rdtsc(), - __ATOMIC_SEQ_CST); + atomic_store(&queue->start_times[idx % SPINNING_QUEUE_SIZE], rdtsc()); return true; } @@ -148,7 +146,7 @@ static bool spinning_queue_push(void) { static void spinning_queue_pop() { struct spinning_queue *queue = __export_spinning_queue_addr; - __atomic_add_fetch(&queue->end, -1, __ATOMIC_SEQ_CST); + atomic_sub(&queue->end, 1); } // spinning_queue_remove_first removes one thread from a queue that has been @@ -162,18 +160,15 @@ static bool spinning_queue_remove_first(uint64_t timeout) { uint64_t ts; uint32_t idx; - idx = __atomic_load_n(&queue->start, __ATOMIC_SEQ_CST); - ts = __atomic_load_n(&queue->start_times[idx % SPINNING_QUEUE_SIZE], - __ATOMIC_SEQ_CST); + idx = atomic_load(&queue->start); + ts = atomic_load(&queue->start_times[idx % SPINNING_QUEUE_SIZE]); if (ts == 0 || rdtsc() - ts < timeout) return false; // The current thread is still in a queue and the length of the queue is twice // of the maximum number of threads, so we can zero the element and be sure // that nobody is trying to set it in a non-zero value. - __atomic_store_n(&queue->start_times[idx % SPINNING_QUEUE_SIZE], 0, - __ATOMIC_SEQ_CST); - if (!__atomic_compare_exchange_n(&queue->start, &idx, idx + 1, false, - __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST)) { + atomic_store(&queue->start_times[idx % SPINNING_QUEUE_SIZE], 0); + if (!atomic_compare_exchange(&queue->start, &idx, idx + 1)) { return false; } @@ -188,29 +183,29 @@ struct thread_context *queue_get_context(struct sysmsg *sysmsg) { MAX_CONTEXT_QUEUE_ENTRIES - 1); while (!is_empty(queue)) { - uint64_t idx = __atomic_load_n(&queue->start, __ATOMIC_ACQUIRE); + uint64_t idx = atomic_load(&queue->start); uint32_t next = idx % MAX_CONTEXT_QUEUE_ENTRIES; - uint64_t v = __atomic_load_n(&queue->ringbuffer[next], __ATOMIC_ACQUIRE); + uint64_t v = atomic_load(&queue->ringbuffer[next]); // We need to check the index to be sure that a ring buffer hasn't been // recycled. if ((v >> CQ_INDEX_SHIFT) != idx) continue; - if (!__atomic_compare_exchange_n(&queue->ringbuffer[next], &v, - INVALID_CONTEXT_ID, false, - __ATOMIC_ACQ_REL, __ATOMIC_ACQUIRE)) + if (!atomic_compare_exchange(&queue->ringbuffer[next], &v, + INVALID_CONTEXT_ID)) { continue; + } uint32_t context_id = v & CQ_CONTEXT_MASK; if (context_id == INVALID_CONTEXT_ID) continue; - __atomic_add_fetch(&queue->start, 1, __ATOMIC_ACQ_REL); + atomic_add(&queue->start, 1); if (context_id > MAX_GUEST_CONTEXTS) { panic(context_id); } struct thread_context *ctx = thread_context_addr(context_id); sysmsg->context = ctx; - __atomic_store_n(&ctx->acked, 1, __ATOMIC_RELEASE); - __atomic_store_n(&ctx->thread_id, sysmsg->thread_id, __ATOMIC_RELEASE); + atomic_store(&ctx->acked, 1); + atomic_store(&ctx->thread_id, sysmsg->thread_id); return ctx; } return NULL; @@ -232,26 +227,22 @@ static struct thread_context *get_context_fast(struct sysmsg *sysmsg, ctx = queue_get_context(sysmsg); if (ctx) { - __atomic_store_n(&queue->fast_path_failed_in_row, 0, __ATOMIC_RELEASE); + atomic_store(&queue->fast_path_failed_in_row, 0); spinning_queue_pop(); return ctx; } - if (__atomic_load_n(&queue->fast_path_disabled, __ATOMIC_ACQUIRE) != 0 && + if (atomic_load(&queue->fast_path_disabled) != 0 && spinning_queue_remove_first(0)) { break; } - nr_active_threads = - __atomic_load_n(&queue->num_active_threads, __ATOMIC_ACQUIRE); - nr_awake_contexts = - __atomic_load_n(&queue->num_awake_contexts, __ATOMIC_ACQUIRE); + nr_active_threads = atomic_load(&queue->num_active_threads); + nr_awake_contexts = atomic_load(&queue->num_awake_contexts); if (nr_awake_contexts < nr_active_threads) { - if (__atomic_compare_exchange_n(&queue->num_active_threads, - &nr_active_threads, nr_active_threads - 1, - false, __ATOMIC_SEQ_CST, - __ATOMIC_SEQ_CST)) { + if (atomic_compare_exchange(&queue->num_active_threads, + &nr_active_threads, nr_active_threads - 1)) { nr_active_threads -= 1; if (spinning_queue_remove_first(0)) { *nr_active_threads_p = nr_active_threads; @@ -260,16 +251,14 @@ static struct thread_context *get_context_fast(struct sysmsg *sysmsg, // spinning_queue_remove_first can fail due to a race with another // thread. - __atomic_add_fetch(&queue->num_active_threads, 1, __ATOMIC_ACQ_REL); + atomic_add(&queue->num_active_threads, 1); } } if (spinning_queue_remove_first(__export_deep_sleep_timeout)) { - uint32_t nr = __atomic_add_fetch(&queue->fast_path_failed_in_row, 1, - __ATOMIC_ACQ_REL); + uint32_t nr = atomic_add(&queue->fast_path_failed_in_row, 1); if (nr >= FAILED_FAST_PATH_LIMIT) { - __atomic_store_n(&queue->fast_path_disalbed_ts, rdtsc(), - __ATOMIC_RELEASE); + atomic_store(&queue->fast_path_disalbed_ts, rdtsc()); } break; } @@ -290,26 +279,25 @@ struct thread_context *get_context(struct sysmsg *sysmsg) { struct thread_context *ctx; // Change sysmsg thread state just to indicate thread is not asleep. - __atomic_store_n(&sysmsg->state, THREAD_STATE_PREP, __ATOMIC_RELEASE); + atomic_store(&sysmsg->state, THREAD_STATE_PREP); ctx = queue_get_context(sysmsg); if (ctx) { - __atomic_store_n(&queue->fast_path_failed_in_row, 0, __ATOMIC_RELEASE); + atomic_store(&queue->fast_path_failed_in_row, 0); return ctx; } - uint64_t slow_path_ts = - __atomic_load_n(&queue->fast_path_disalbed_ts, __ATOMIC_ACQUIRE); + uint64_t slow_path_ts = atomic_load(&queue->fast_path_disalbed_ts); bool fast_path_enabled = true; if (!slow_path_ts) { if (rdtsc() - slow_path_ts > FAILED_FAST_PATH_TIMEOUT) { - __atomic_store_n(&queue->fast_path_failed_in_row, 0, __ATOMIC_RELEASE); - __atomic_store_n(&queue->fast_path_disalbed_ts, 0, __ATOMIC_RELEASE); + atomic_store(&queue->fast_path_failed_in_row, 0); + atomic_store(&queue->fast_path_disalbed_ts, 0); } else { fast_path_enabled = false; } } - if (__atomic_load_n(&queue->fast_path_disabled, __ATOMIC_ACQUIRE) != 0) { + if (atomic_load(&queue->fast_path_disabled) != 0) { fast_path_enabled = false; } @@ -319,13 +307,11 @@ struct thread_context *get_context(struct sysmsg *sysmsg) { if (ctx) return ctx; } if (nr_active_threads == NR_IF_THREAD_IS_ACTIVE) { - nr_active_threads = - __atomic_sub_fetch(&queue->num_active_threads, 1, __ATOMIC_ACQ_REL); + nr_active_threads = atomic_sub(&queue->num_active_threads, 1); } - __atomic_store_n(&sysmsg->state, THREAD_STATE_ASLEEP, __ATOMIC_RELEASE); - uint32_t nr_active_contexts = - __atomic_load_n(&queue->num_active_contexts, __ATOMIC_ACQUIRE); + atomic_store(&sysmsg->state, THREAD_STATE_ASLEEP); + uint32_t nr_active_contexts = atomic_load(&queue->num_active_contexts); // We have to make another attempt to get a context here to prevent TOCTTOU // races with waitOnState and kickSysmsgThread. There are two assumptions: // * If the queue isn't empty, one or more threads have to be active. @@ -334,17 +320,16 @@ struct thread_context *get_context(struct sysmsg *sysmsg) { if (nr_active_threads == 0 || nr_active_threads < nr_active_contexts) { ctx = queue_get_context(sysmsg); if (ctx) { - __atomic_store_n(&sysmsg->state, THREAD_STATE_PREP, __ATOMIC_RELEASE); - __atomic_add_fetch(&queue->num_active_threads, 1, __ATOMIC_ACQ_REL); + atomic_store(&sysmsg->state, THREAD_STATE_PREP); + atomic_add(&queue->num_active_threads, 1); return ctx; } } - while (__atomic_load_n(&sysmsg->state, __ATOMIC_ACQUIRE) == - THREAD_STATE_ASLEEP) { + while (atomic_load(&sysmsg->state) == THREAD_STATE_ASLEEP) { sys_futex(&sysmsg->state, FUTEX_WAIT, THREAD_STATE_ASLEEP, NULL, NULL, 0); } - __atomic_add_fetch(&queue->num_active_threads, 1, __ATOMIC_ACQ_REL); + atomic_add(&queue->num_active_threads, 1); } } @@ -356,11 +341,11 @@ struct thread_context *switch_context(struct sysmsg *sysmsg, struct context_queue *queue = __export_context_queue_addr; if (ctx) { - __atomic_sub_fetch(&queue->num_active_contexts, 1, __ATOMIC_ACQ_REL); - __atomic_store_n(&ctx->thread_id, INVALID_THREAD_ID, __ATOMIC_RELEASE); - __atomic_store_n(&ctx->last_thread_id, sysmsg->thread_id, __ATOMIC_RELEASE); - __atomic_store_n(&ctx->state, new_context_state, __ATOMIC_RELEASE); - if (__atomic_load_n(&ctx->sentry_fast_path, __ATOMIC_ACQUIRE) == 0) { + atomic_sub(&queue->num_active_contexts, 1); + atomic_store(&ctx->thread_id, INVALID_THREAD_ID); + atomic_store(&ctx->last_thread_id, sysmsg->thread_id); + atomic_store(&ctx->state, new_context_state); + if (atomic_load(&ctx->sentry_fast_path) == 0) { int ret = sys_futex(&ctx->state, FUTEX_WAKE, 1, NULL, NULL, 0); if (ret < 0) { panic(ret);