diff options
author | Richard Henderson <richard.henderson@linaro.org> | 2023-07-16 16:54:16 +0000 |
---|---|---|
committer | Richard Henderson <richard.henderson@linaro.org> | 2023-07-23 17:57:10 +0100 |
commit | 2c8412d469707124c6f471822aff2e2cab395907 (patch) | |
tree | 30373a4755be4842e9949c959aefb39f7893a77a | |
parent | f1ce0b80283e19d5e65a5c2b0b4de6d92ca7f527 (diff) | |
download | qemu-2c8412d469707124c6f471822aff2e2cab395907.zip qemu-2c8412d469707124c6f471822aff2e2cab395907.tar.gz qemu-2c8412d469707124c6f471822aff2e2cab395907.tar.bz2 |
accel/tcg: Take mmap_lock in load_atomic*_or_exit
For user-only, the probe for page writability may race with another
thread's mprotect. Take the mmap_lock around the operation. This
is still faster than the start/end_exclusive fallback.
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
-rw-r--r-- | accel/tcg/ldst_atomicity.c.inc | 32 |
1 files changed, 18 insertions, 14 deletions
diff --git a/accel/tcg/ldst_atomicity.c.inc b/accel/tcg/ldst_atomicity.c.inc index de70531..e5c590a 100644 --- a/accel/tcg/ldst_atomicity.c.inc +++ b/accel/tcg/ldst_atomicity.c.inc @@ -159,9 +159,11 @@ static uint64_t load_atomic8_or_exit(CPUArchState *env, uintptr_t ra, void *pv) * another process, because the fallback start_exclusive solution * provides no protection across processes. */ - if (!page_check_range(h2g(pv), 8, PAGE_WRITE_ORG)) { - uint64_t *p = __builtin_assume_aligned(pv, 8); - return *p; + WITH_MMAP_LOCK_GUARD() { + if (!page_check_range(h2g(pv), 8, PAGE_WRITE_ORG)) { + uint64_t *p = __builtin_assume_aligned(pv, 8); + return *p; + } } #endif @@ -186,25 +188,27 @@ static Int128 load_atomic16_or_exit(CPUArchState *env, uintptr_t ra, void *pv) return atomic16_read_ro(p); } -#ifdef CONFIG_USER_ONLY /* * We can only use cmpxchg to emulate a load if the page is writable. * If the page is not writable, then assume the value is immutable * and requires no locking. This ignores the case of MAP_SHARED with * another process, because the fallback start_exclusive solution * provides no protection across processes. + * + * In system mode all guest pages are writable. For user mode, + * we must take mmap_lock so that the query remains valid until + * the write is complete -- tests/tcg/multiarch/munmap-pthread.c + * is an example that can race. */ - if (!page_check_range(h2g(p), 16, PAGE_WRITE_ORG)) { - return *p; - } + WITH_MMAP_LOCK_GUARD() { +#ifdef CONFIG_USER_ONLY + if (!page_check_range(h2g(p), 16, PAGE_WRITE_ORG)) { + return *p; + } #endif - - /* - * In system mode all guest pages are writable, and for user-only - * we have just checked writability. Try cmpxchg. - */ - if (HAVE_ATOMIC128_RW) { - return atomic16_read_rw(p); + if (HAVE_ATOMIC128_RW) { + return atomic16_read_rw(p); + } } /* Ultimate fallback: re-execute in serial context. */ |