translate_insn() implementations fetch instruction bytes piecemeal,
which can cause qemu-user to generate inconsistent translations if
another thread modifies them concurrently [1].
Fix by making pages containing translated instruction non-writable
right before loading instruction bytes from them.
[1] https://lists.nongnu.org/archive/html/qemu-devel/2021-08/msg00644.html
Signed-off-by: Ilya Leoshkevich
---
v2: https://lists.nongnu.org/archive/html/qemu-devel/2021-08/msg00819.html
v2 -> v3: Move translator_ld*_swap() functions from translator.h into
translator.c for a better size trade-off (Richard).
accel/tcg/translate-all.c | 59 +--
accel/tcg/translator.c| 30
include/exec/translate-all.h | 1 +
include/exec/translator.h | 52 +---
target/alpha/translate.c | 2 +-
target/arm/arm_ldst.h | 12 ++---
target/arm/translate-a64.c| 2 +-
target/arm/translate.c| 9 ++--
target/hexagon/translate.c| 3 +-
target/hppa/translate.c | 2 +-
target/i386/tcg/translate.c | 10 ++--
target/m68k/translate.c | 2 +-
target/mips/tcg/micromips_translate.c.inc | 2 +-
target/mips/tcg/mips16e_translate.c.inc | 4 +-
target/mips/tcg/nanomips_translate.c.inc | 4 +-
target/mips/tcg/translate.c | 8 +--
target/openrisc/translate.c | 2 +-
target/ppc/translate.c| 5 +-
target/riscv/translate.c | 5 +-
target/s390x/tcg/translate.c | 16 +++---
target/sh4/translate.c| 4 +-
target/sparc/translate.c | 2 +-
target/xtensa/translate.c | 5 +-
23 files changed, 153 insertions(+), 88 deletions(-)
diff --git a/accel/tcg/translate-all.c b/accel/tcg/translate-all.c
index bbfcfb698c..fb9ebfad9e 100644
--- a/accel/tcg/translate-all.c
+++ b/accel/tcg/translate-all.c
@@ -1297,31 +1297,8 @@ static inline void tb_page_add(PageDesc *p,
TranslationBlock *tb,
invalidate_page_bitmap(p);
#if defined(CONFIG_USER_ONLY)
-if (p->flags & PAGE_WRITE) {
-target_ulong addr;
-PageDesc *p2;
-int prot;
-
-/* force the host page as non writable (writes will have a
- page fault + mprotect overhead) */
-page_addr &= qemu_host_page_mask;
-prot = 0;
-for (addr = page_addr; addr < page_addr + qemu_host_page_size;
-addr += TARGET_PAGE_SIZE) {
-
-p2 = page_find(addr >> TARGET_PAGE_BITS);
-if (!p2) {
-continue;
-}
-prot |= p2->flags;
-p2->flags &= ~PAGE_WRITE;
- }
-mprotect(g2h_untagged(page_addr), qemu_host_page_size,
- (prot & PAGE_BITS) & ~PAGE_WRITE);
-if (DEBUG_TB_INVALIDATE_GATE) {
-printf("protecting code page: 0x" TB_PAGE_ADDR_FMT "\n",
page_addr);
-}
-}
+/* translator_loop() must have made all TB pages non-writable */
+assert(!(p->flags & PAGE_WRITE));
#else
/* if some code is already present, then the pages are already
protected. So we handle the case where only the first TB is
@@ -2394,6 +2371,38 @@ int page_check_range(target_ulong start, target_ulong
len, int flags)
return 0;
}
+void page_protect(tb_page_addr_t page_addr)
+{
+target_ulong addr;
+PageDesc *p;
+int prot;
+
+p = page_find(page_addr >> TARGET_PAGE_BITS);
+if (p && (p->flags & PAGE_WRITE)) {
+/*
+ * Force the host page as non writable (writes will have a page fault +
+ * mprotect overhead).
+ */
+page_addr &= qemu_host_page_mask;
+prot = 0;
+for (addr = page_addr; addr < page_addr + qemu_host_page_size;
+ addr += TARGET_PAGE_SIZE) {
+
+p = page_find(addr >> TARGET_PAGE_BITS);
+if (!p) {
+continue;
+}
+prot |= p->flags;
+p->flags &= ~PAGE_WRITE;
+}
+mprotect(g2h_untagged(page_addr), qemu_host_page_size,
+ (prot & PAGE_BITS) & ~PAGE_WRITE);
+if (DEBUG_TB_INVALIDATE_GATE) {
+printf("protecting code page: 0x" TB_PAGE_ADDR_FMT "\n",
page_addr);
+}
+}
+}
+
/* called from signal handler: invalidate the code and unprotect the
* page. Return 0 if the fault was not handled, 1 if it was handled,
* and 2 if it was handled but the caller must cause the TB to be
diff --git a/accel/tcg/translator.c b/accel/tcg/translator.c
index c53a7f8e44..93423949e7 100644
--- a/accel/tcg/translator.c
+++ b/accel/tcg/translator.c
@@ -56,6 +56,7 @@ void translator_loop(const TranslatorOps *ops,
DisasContextBase *db,
db->num_insns = 0;
db->max_insns = max_insns;
db->singlestep_enabled = cflags &
