accel/tcg: Clear PAGE_WRITE before translation
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 <iii@linux.ibm.com> Message-Id: <20210805204835.158918-1-iii@linux.ibm.com> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
This commit is contained in:
parent
4e116893c6
commit
f025692c99
@ -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
|
||||
|
@ -42,6 +42,15 @@ bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest)
|
||||
return ((db->pc_first ^ dest) & TARGET_PAGE_MASK) == 0;
|
||||
}
|
||||
|
||||
static inline void translator_page_protect(DisasContextBase *dcbase,
|
||||
target_ulong pc)
|
||||
{
|
||||
#ifdef CONFIG_USER_ONLY
|
||||
dcbase->page_protect_end = pc | ~TARGET_PAGE_MASK;
|
||||
page_protect(pc);
|
||||
#endif
|
||||
}
|
||||
|
||||
void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
|
||||
CPUState *cpu, TranslationBlock *tb, int max_insns)
|
||||
{
|
||||
@ -56,6 +65,7 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
|
||||
db->num_insns = 0;
|
||||
db->max_insns = max_insns;
|
||||
db->singlestep_enabled = cflags & CF_SINGLE_STEP;
|
||||
translator_page_protect(db, db->pc_next);
|
||||
|
||||
ops->init_disas_context(db, cpu);
|
||||
tcg_debug_assert(db->is_jmp == DISAS_NEXT); /* no early exit */
|
||||
@ -137,3 +147,32 @@ void translator_loop(const TranslatorOps *ops, DisasContextBase *db,
|
||||
}
|
||||
#endif
|
||||
}
|
||||
|
||||
static inline void translator_maybe_page_protect(DisasContextBase *dcbase,
|
||||
target_ulong pc, size_t len)
|
||||
{
|
||||
#ifdef CONFIG_USER_ONLY
|
||||
target_ulong end = pc + len - 1;
|
||||
|
||||
if (end > dcbase->page_protect_end) {
|
||||
translator_page_protect(dcbase, end);
|
||||
}
|
||||
#endif
|
||||
}
|
||||
|
||||
#define GEN_TRANSLATOR_LD(fullname, type, load_fn, swap_fn) \
|
||||
type fullname ## _swap(CPUArchState *env, DisasContextBase *dcbase, \
|
||||
abi_ptr pc, bool do_swap) \
|
||||
{ \
|
||||
translator_maybe_page_protect(dcbase, pc, sizeof(type)); \
|
||||
type ret = load_fn(env, pc); \
|
||||
if (do_swap) { \
|
||||
ret = swap_fn(ret); \
|
||||
} \
|
||||
plugin_insn_append(&ret, sizeof(ret)); \
|
||||
return ret; \
|
||||
}
|
||||
|
||||
FOR_EACH_TRANSLATOR_LD(GEN_TRANSLATOR_LD)
|
||||
|
||||
#undef GEN_TRANSLATOR_LD
|
||||
|
@ -33,6 +33,7 @@ void tb_invalidate_phys_page_range(tb_page_addr_t start, tb_page_addr_t end);
|
||||
void tb_check_watchpoint(CPUState *cpu, uintptr_t retaddr);
|
||||
|
||||
#ifdef CONFIG_USER_ONLY
|
||||
void page_protect(tb_page_addr_t page_addr);
|
||||
int page_unprotect(target_ulong address, uintptr_t pc);
|
||||
#endif
|
||||
|
||||
|
@ -23,6 +23,7 @@
|
||||
#include "exec/exec-all.h"
|
||||
#include "exec/cpu_ldst.h"
|
||||
#include "exec/plugin-gen.h"
|
||||
#include "exec/translate-all.h"
|
||||
#include "tcg/tcg.h"
|
||||
|
||||
|
||||
@ -74,6 +75,17 @@ typedef struct DisasContextBase {
|
||||
int num_insns;
|
||||
int max_insns;
|
||||
bool singlestep_enabled;
|
||||
#ifdef CONFIG_USER_ONLY
|
||||
/*
|
||||
* Guest address of the last byte of the last protected page.
|
||||
*
|
||||
* Pages containing the translated instructions are made non-writable in
|
||||
* order to achieve consistency in case another thread is modifying the
|
||||
* code while translate_insn() fetches the instruction bytes piecemeal.
|
||||
* Such writer threads are blocked on mmap_lock() in page_unprotect().
|
||||
*/
|
||||
target_ulong page_protect_end;
|
||||
#endif
|
||||
} DisasContextBase;
|
||||
|
||||
/**
|
||||
@ -156,28 +168,23 @@ bool translator_use_goto_tb(DisasContextBase *db, target_ulong dest);
|
||||
*/
|
||||
|
||||
#define GEN_TRANSLATOR_LD(fullname, type, load_fn, swap_fn) \
|
||||
static inline type \
|
||||
fullname ## _swap(CPUArchState *env, DisasContextBase *dcbase, \
|
||||
abi_ptr pc, bool do_swap) \
|
||||
{ \
|
||||
type ret = load_fn(env, pc); \
|
||||
if (do_swap) { \
|
||||
ret = swap_fn(ret); \
|
||||
} \
|
||||
plugin_insn_append(&ret, sizeof(ret)); \
|
||||
return ret; \
|
||||
} \
|
||||
type fullname ## _swap(CPUArchState *env, DisasContextBase *dcbase, \
|
||||
abi_ptr pc, bool do_swap); \
|
||||
static inline type fullname(CPUArchState *env, \
|
||||
DisasContextBase *dcbase, abi_ptr pc) \
|
||||
{ \
|
||||
return fullname ## _swap(env, dcbase, pc, false); \
|
||||
}
|
||||
|
||||
GEN_TRANSLATOR_LD(translator_ldub, uint8_t, cpu_ldub_code, /* no swap */)
|
||||
GEN_TRANSLATOR_LD(translator_ldsw, int16_t, cpu_ldsw_code, bswap16)
|
||||
GEN_TRANSLATOR_LD(translator_lduw, uint16_t, cpu_lduw_code, bswap16)
|
||||
GEN_TRANSLATOR_LD(translator_ldl, uint32_t, cpu_ldl_code, bswap32)
|
||||
GEN_TRANSLATOR_LD(translator_ldq, uint64_t, cpu_ldq_code, bswap64)
|
||||
#define FOR_EACH_TRANSLATOR_LD(F) \
|
||||
F(translator_ldub, uint8_t, cpu_ldub_code, /* no swap */) \
|
||||
F(translator_ldsw, int16_t, cpu_ldsw_code, bswap16) \
|
||||
F(translator_lduw, uint16_t, cpu_lduw_code, bswap16) \
|
||||
F(translator_ldl, uint32_t, cpu_ldl_code, bswap32) \
|
||||
F(translator_ldq, uint64_t, cpu_ldq_code, bswap64)
|
||||
|
||||
FOR_EACH_TRANSLATOR_LD(GEN_TRANSLATOR_LD)
|
||||
|
||||
#undef GEN_TRANSLATOR_LD
|
||||
|
||||
#endif /* EXEC__TRANSLATOR_H */
|
||||
|
Loading…
Reference in New Issue
Block a user