* [PATCH v5 0/4] linux-user: Fix siginfo_t contents when jumping to non-readable pages
@ 2022-08-17 15:05 Ilya Leoshkevich
2022-08-17 15:05 ` [PATCH v5 1/4] linux-user: Clear tb_jmp_cache on mprotect() Ilya Leoshkevich
` (5 more replies)
0 siblings, 6 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2022-08-17 15:05 UTC (permalink / raw)
To: Laurent Vivier, Eduardo Habkost, Marcel Apfelbaum,
Philippe Mathieu-Daudé,
Yanan Wang, Richard Henderson, Paolo Bonzini, David Hildenbrand,
Peter Maydell
Cc: qemu-devel, qemu-s390x, Christian Borntraeger, Ilya Leoshkevich
Hi,
I noticed that when we get a SEGV due to jumping to non-readable
memory, sometimes si_addr and program counter in siginfo_t are slightly
off. I tracked this down to the assumption that translators stop before
the end of a page, while in reality they may stop right after it.
Patch 1 fixes an invalidation issue, which may prevent SEGV from
happening altogether.
Patches 2-3 fix the main issue on x86_64 and s390x. Many other
architectures have fixed-size instructions and are not affected.
Patch 4 adds tests.
Note: this series depends on [1].
Best regards,
Ilya
v1: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg00822.html
v1 -> v2: Fix individual translators instead of translator_loop
(Peter).
v2: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg01079.html
v2 -> v3: Peek at the next instruction on s390x (Richard).
Undo more on i386 (Richard).
Check PAGE_EXEC, not PAGE_READ (Peter, Richard).
v3: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg01306.html
v3 -> v4: Improve the commit message in patch 1 to better reflect what
exactly is being fixed there.
Factor out the is_same_page() patch (Richard).
Do not touch the common code in the i386 fix (Richard).
v4: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg01747.html
v4 -> v5: Drop patch 2.
Use a different fix for the invalidation issue based on
discussion with Richard [2].
[1] https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg02472.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg02556.html
Ilya Leoshkevich (4):
linux-user: Clear tb_jmp_cache on mprotect()
target/s390x: Make translator stop before the end of a page
target/i386: Make translator stop before the end of a page
tests/tcg: Test siginfo_t contents when jumping to non-readable pages
linux-user/mmap.c | 14 +++
target/i386/tcg/translate.c | 25 +++++-
target/s390x/tcg/translate.c | 15 +++-
tests/tcg/multiarch/noexec.h | 114 ++++++++++++++++++++++++
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/noexec.c | 145 +++++++++++++++++++++++++++++++
tests/tcg/x86_64/Makefile.target | 3 +-
tests/tcg/x86_64/noexec.c | 116 +++++++++++++++++++++++++
8 files changed, 427 insertions(+), 6 deletions(-)
create mode 100644 tests/tcg/multiarch/noexec.h
create mode 100644 tests/tcg/s390x/noexec.c
create mode 100644 tests/tcg/x86_64/noexec.c
--
2.37.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v5 1/4] linux-user: Clear tb_jmp_cache on mprotect()
2022-08-17 15:05 [PATCH v5 0/4] linux-user: Fix siginfo_t contents when jumping to non-readable pages Ilya Leoshkevich
@ 2022-08-17 15:05 ` Ilya Leoshkevich
2022-08-17 15:05 ` [PATCH v5 2/4] target/s390x: Make translator stop before the end of a page Ilya Leoshkevich
` (4 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2022-08-17 15:05 UTC (permalink / raw)
To: Laurent Vivier, Eduardo Habkost, Marcel Apfelbaum,
Philippe Mathieu-Daudé,
Yanan Wang, Richard Henderson, Paolo Bonzini, David Hildenbrand,
Peter Maydell
Cc: qemu-devel, qemu-s390x, Christian Borntraeger, Ilya Leoshkevich
Currently it's possible to execute pages that do not have PAGE_EXEC if
there is an existing translation block. Fix by clearing tb_jmp_cache,
which forces HELPER(lookup_tb_ptr)() to recheck permission bits the
next time.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
linux-user/mmap.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/linux-user/mmap.c b/linux-user/mmap.c
index bbc674311b..bd96c876ba 100644
--- a/linux-user/mmap.c
+++ b/linux-user/mmap.c
@@ -115,6 +115,7 @@ int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
{
abi_ulong end, host_start, host_end, addr;
int prot1, ret, page_flags, host_prot;
+ CPUState *cpu;
trace_target_mprotect(start, len, target_prot);
@@ -178,6 +179,19 @@ int target_mprotect(abi_ulong start, abi_ulong len, int target_prot)
}
}
page_set_flags(start, start + len, page_flags);
+
+ /*
+ * Unlike target_mmap(), target_munmap() and target_mremap(), we don't need
+ * to call tb_invalidate_phys_range() here, since pages still hold the same
+ * data. However, tb_jmp_cache needs to be cleared, otherwise after
+ * clearing PAGE_EXEC it would still be possible to jump to the existing
+ * translation blocks. We just clear the whole cache here: mprotect() is
+ * rare enough, so a more fine-grained approach is not necessary.
+ */
+ CPU_FOREACH(cpu) {
+ cpu_tb_jmp_cache_clear(cpu);
+ }
+
mmap_unlock();
return 0;
error:
--
2.37.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 2/4] target/s390x: Make translator stop before the end of a page
2022-08-17 15:05 [PATCH v5 0/4] linux-user: Fix siginfo_t contents when jumping to non-readable pages Ilya Leoshkevich
2022-08-17 15:05 ` [PATCH v5 1/4] linux-user: Clear tb_jmp_cache on mprotect() Ilya Leoshkevich
@ 2022-08-17 15:05 ` Ilya Leoshkevich
2022-08-17 15:05 ` [PATCH v5 3/4] target/i386: " Ilya Leoshkevich
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2022-08-17 15:05 UTC (permalink / raw)
To: Laurent Vivier, Eduardo Habkost, Marcel Apfelbaum,
Philippe Mathieu-Daudé,
Yanan Wang, Richard Henderson, Paolo Bonzini, David Hildenbrand,
Peter Maydell
Cc: qemu-devel, qemu-s390x, Christian Borntraeger, Ilya Leoshkevich
Right now translator stops right *after* the end of a page, which
breaks reporting of fault locations when the last instruction of a
multi-insn translation block crosses a page boundary.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
target/s390x/tcg/translate.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c
index d4c0b9b3a2..1d2dddab1c 100644
--- a/target/s390x/tcg/translate.c
+++ b/target/s390x/tcg/translate.c
@@ -6609,6 +6609,14 @@ static void s390x_tr_insn_start(DisasContextBase *dcbase, CPUState *cs)
dc->insn_start = tcg_last_op();
}
+static target_ulong get_next_pc(CPUS390XState *env, DisasContext *s,
+ uint64_t pc)
+{
+ uint64_t insn = ld_code2(env, s, pc);
+
+ return pc + get_ilen((insn >> 8) & 0xff);
+}
+
static void s390x_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
{
CPUS390XState *env = cs->env_ptr;
@@ -6616,10 +6624,9 @@ static void s390x_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs)
dc->base.is_jmp = translate_one(env, dc);
if (dc->base.is_jmp == DISAS_NEXT) {
- uint64_t page_start;
-
- page_start = dc->base.pc_first & TARGET_PAGE_MASK;
- if (dc->base.pc_next - page_start >= TARGET_PAGE_SIZE || dc->ex_value) {
+ if (!is_same_page(dcbase, dc->base.pc_next) ||
+ !is_same_page(dcbase, get_next_pc(env, dc, dc->base.pc_next)) ||
+ dc->ex_value) {
dc->base.is_jmp = DISAS_TOO_MANY;
}
}
--
2.37.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 3/4] target/i386: Make translator stop before the end of a page
2022-08-17 15:05 [PATCH v5 0/4] linux-user: Fix siginfo_t contents when jumping to non-readable pages Ilya Leoshkevich
2022-08-17 15:05 ` [PATCH v5 1/4] linux-user: Clear tb_jmp_cache on mprotect() Ilya Leoshkevich
2022-08-17 15:05 ` [PATCH v5 2/4] target/s390x: Make translator stop before the end of a page Ilya Leoshkevich
@ 2022-08-17 15:05 ` Ilya Leoshkevich
2022-08-17 15:05 ` [PATCH v5 4/4] tests/tcg: Test siginfo_t contents when jumping to non-readable pages Ilya Leoshkevich
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2022-08-17 15:05 UTC (permalink / raw)
To: Laurent Vivier, Eduardo Habkost, Marcel Apfelbaum,
Philippe Mathieu-Daudé,
Yanan Wang, Richard Henderson, Paolo Bonzini, David Hildenbrand,
Peter Maydell
Cc: qemu-devel, qemu-s390x, Christian Borntraeger, Ilya Leoshkevich
Right now translator stops right *after* the end of a page, which
breaks reporting of fault locations when the last instruction of a
multi-insn translation block crosses a page boundary.
An implementation, like the one arm and s390x have, would require an
i386 length disassembler, which is burdensome to maintain. Another
alternative would be to single-step at the end of a guest page, but
this may come with a performance impact.
Fix by snapshotting disassembly state and restoring it after we figure
out we crossed a page boundary. This includes rolling back cc_op
updates and emitted ops.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1143
---
target/i386/tcg/translate.c | 25 ++++++++++++++++++++++++-
1 file changed, 24 insertions(+), 1 deletion(-)
diff --git a/target/i386/tcg/translate.c b/target/i386/tcg/translate.c
index 4836c889e0..6481ae5c24 100644
--- a/target/i386/tcg/translate.c
+++ b/target/i386/tcg/translate.c
@@ -130,6 +130,7 @@ typedef struct DisasContext {
TCGv_i64 tmp1_i64;
sigjmp_buf jmpbuf;
+ TCGOp *prev_insn_end;
} DisasContext;
/* The environment in which user-only runs is constrained. */
@@ -2008,6 +2009,12 @@ static uint64_t advance_pc(CPUX86State *env, DisasContext *s, int num_bytes)
{
uint64_t pc = s->pc;
+ /* This is a subsequent insn that crosses a page boundary. */
+ if (s->base.num_insns > 1 &&
+ !is_same_page(&s->base, s->pc + num_bytes - 1)) {
+ siglongjmp(s->jmpbuf, 2);
+ }
+
s->pc += num_bytes;
if (unlikely(s->pc - s->pc_start > X86_MAX_INSN_LENGTH)) {
/* If the instruction's 16th byte is on a different page than the 1st, a
@@ -4556,6 +4563,8 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
int modrm, reg, rm, mod, op, opreg, val;
target_ulong next_eip, tval;
target_ulong pc_start = s->base.pc_next;
+ bool orig_cc_op_dirty = s->cc_op_dirty;
+ CCOp orig_cc_op = s->cc_op;
s->pc_start = s->pc = pc_start;
s->override = -1;
@@ -4568,9 +4577,22 @@ static target_ulong disas_insn(DisasContext *s, CPUState *cpu)
s->rip_offset = 0; /* for relative ip address */
s->vex_l = 0;
s->vex_v = 0;
- if (sigsetjmp(s->jmpbuf, 0) != 0) {
+ switch (sigsetjmp(s->jmpbuf, 0)) {
+ case 0:
+ break;
+ case 1:
gen_exception_gpf(s);
return s->pc;
+ case 2:
+ /* Restore state that may affect the next instruction. */
+ s->cc_op_dirty = orig_cc_op_dirty;
+ s->cc_op = orig_cc_op;
+ s->base.num_insns--;
+ tcg_remove_ops_after(s->prev_insn_end);
+ s->base.is_jmp = DISAS_TOO_MANY;
+ return pc_start;
+ default:
+ g_assert_not_reached();
}
prefixes = 0;
@@ -8632,6 +8654,7 @@ static void i386_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu)
{
DisasContext *dc = container_of(dcbase, DisasContext, base);
+ dc->prev_insn_end = tcg_last_op();
tcg_gen_insn_start(dc->base.pc_next, dc->cc_op);
}
--
2.37.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v5 4/4] tests/tcg: Test siginfo_t contents when jumping to non-readable pages
2022-08-17 15:05 [PATCH v5 0/4] linux-user: Fix siginfo_t contents when jumping to non-readable pages Ilya Leoshkevich
` (2 preceding siblings ...)
2022-08-17 15:05 ` [PATCH v5 3/4] target/i386: " Ilya Leoshkevich
@ 2022-08-17 15:05 ` Ilya Leoshkevich
2022-08-17 16:23 ` [PATCH v5 0/4] linux-user: Fix " Richard Henderson
2022-08-18 16:55 ` Vivian Wang
5 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2022-08-17 15:05 UTC (permalink / raw)
To: Laurent Vivier, Eduardo Habkost, Marcel Apfelbaum,
Philippe Mathieu-Daudé,
Yanan Wang, Richard Henderson, Paolo Bonzini, David Hildenbrand,
Peter Maydell
Cc: qemu-devel, qemu-s390x, Christian Borntraeger, Ilya Leoshkevich
Add x86_64 and s390x tests to prevent regressions.
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
---
tests/tcg/multiarch/noexec.h | 114 ++++++++++++++++++++++++
tests/tcg/s390x/Makefile.target | 1 +
tests/tcg/s390x/noexec.c | 145 +++++++++++++++++++++++++++++++
tests/tcg/x86_64/Makefile.target | 3 +-
tests/tcg/x86_64/noexec.c | 116 +++++++++++++++++++++++++
5 files changed, 378 insertions(+), 1 deletion(-)
create mode 100644 tests/tcg/multiarch/noexec.h
create mode 100644 tests/tcg/s390x/noexec.c
create mode 100644 tests/tcg/x86_64/noexec.c
diff --git a/tests/tcg/multiarch/noexec.h b/tests/tcg/multiarch/noexec.h
new file mode 100644
index 0000000000..a76e0aa9ea
--- /dev/null
+++ b/tests/tcg/multiarch/noexec.h
@@ -0,0 +1,114 @@
+/*
+ * Common code for arch-specific MMU_INST_FETCH fault testing.
+ *
+ * Declare struct arch_noexec_test before including this file and define
+ * arch_check_mcontext() after that.
+ */
+
+#include <assert.h>
+#include <signal.h>
+#include <stdlib.h>
+#include <string.h>
+#include <sys/mman.h>
+#include <sys/ucontext.h>
+#include <unistd.h>
+
+/* Forward declarations. */
+
+static void arch_check_mcontext(const struct arch_noexec_test *test,
+ const mcontext_t *ctx);
+
+/* Utility functions. */
+
+static void safe_print(const char *s)
+{
+ write(0, s, strlen(s));
+}
+
+static void safe_puts(const char *s)
+{
+ safe_print(s);
+ safe_print("\n");
+}
+
+#define PAGE_ALIGN(p) (void *)((unsigned long)(p) & ~0xfffUL)
+
+/* Testing infrastructure. */
+
+struct noexec_test {
+ const char *name;
+ void (*func)(int);
+ void *page;
+ void *expected_si_addr;
+ struct arch_noexec_test arch;
+};
+
+static const struct noexec_test *current_noexec_test;
+
+static void handle_segv(int sig, siginfo_t *info, void *ucontext)
+{
+ int err;
+
+ if (current_noexec_test == NULL) {
+ safe_puts("[ FAILED ] unexpected SEGV");
+ _exit(1);
+ }
+
+ if (info->si_addr != current_noexec_test->expected_si_addr) {
+ safe_puts("[ FAILED ] wrong si_addr");
+ _exit(1);
+ }
+
+ arch_check_mcontext(¤t_noexec_test->arch,
+ &((ucontext_t *)ucontext)->uc_mcontext);
+
+ err = mprotect(current_noexec_test->page, 0x1000, PROT_READ | PROT_EXEC);
+ if (err != 0) {
+ safe_puts("[ FAILED ] mprotect() failed");
+ _exit(1);
+ }
+
+ current_noexec_test = NULL;
+}
+
+static void test_noexec_1(const struct noexec_test *test)
+{
+ int ret;
+
+ /* Trigger TB creation in order to test invalidation. */
+ test->func(0);
+
+ ret = mprotect(test->page, 0x1000, PROT_NONE);
+ assert(ret == 0);
+
+ /* Trigger SEGV and check that handle_segv() ran. */
+ current_noexec_test = test;
+ test->func(0);
+ assert(current_noexec_test == NULL);
+}
+
+static int test_noexec(struct noexec_test *tests, size_t n_tests)
+{
+ struct sigaction act;
+ size_t i;
+ int err;
+
+ memset(&act, 0, sizeof(act));
+ act.sa_sigaction = handle_segv;
+ act.sa_flags = SA_SIGINFO;
+ err = sigaction(SIGSEGV, &act, NULL);
+ assert(err == 0);
+
+ for (i = 0; i < n_tests; i++) {
+ struct noexec_test *test = &tests[i];
+
+ safe_print("[ RUN ] ");
+ safe_puts(test->name);
+ test_noexec_1(test);
+ safe_puts("[ OK ]");
+ }
+
+ safe_puts("[ PASSED ]");
+
+ return EXIT_SUCCESS;
+}
diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target
index 1a7a4a2f59..5e13a41c3f 100644
--- a/tests/tcg/s390x/Makefile.target
+++ b/tests/tcg/s390x/Makefile.target
@@ -16,6 +16,7 @@ TESTS+=shift
TESTS+=trap
TESTS+=signals-s390x
TESTS+=branch-relative-long
+TESTS+=noexec
Z14_TESTS=vfminmax
vfminmax: LDFLAGS+=-lm
diff --git a/tests/tcg/s390x/noexec.c b/tests/tcg/s390x/noexec.c
new file mode 100644
index 0000000000..2dfc9ee817
--- /dev/null
+++ b/tests/tcg/s390x/noexec.c
@@ -0,0 +1,145 @@
+#define _GNU_SOURCE
+
+struct arch_noexec_test {
+ void *expected_pswa;
+ unsigned long expected_r2;
+};
+
+#include "../multiarch/noexec.h"
+
+static void arch_check_mcontext(const struct arch_noexec_test *test,
+ const mcontext_t *ctx) {
+ if (ctx->psw.addr != (unsigned long)test->expected_pswa) {
+ safe_puts("[ FAILED ] wrong psw.addr");
+ _exit(1);
+ }
+
+ if (ctx->gregs[2] != test->expected_r2) {
+ safe_puts("[ FAILED ] wrong r2");
+ _exit(1);
+ }
+}
+
+#define DEFINE_NX(name, offset) \
+ void name ## _1(int); \
+ void name ## _2(int); \
+ void name ## _exrl(int); \
+ extern const short name ## _end[]; \
+ asm(/* Go to the specified page offset. */ \
+ ".align 0x1000\n" \
+ ".org .+" #offset "\n" \
+ /* %r2 is 0 on entry, overwrite it with 1. */ \
+ ".globl " #name "_1\n" \
+ #name "_1:\n" \
+ ".cfi_startproc\n" \
+ "lgfi %r2,1\n" \
+ /* Overwrite %2 with 2. */ \
+ ".globl " #name "_2\n" \
+ #name "_2:\n" \
+ "lgfi %r2,2\n" \
+ "br %r14\n" \
+ /* End of code. */ \
+ ".globl " #name "_end\n" \
+ #name "_end:\n" \
+ ".cfi_endproc\n" \
+ /* Go to the next page. */ \
+ ".align 0x1000\n" \
+ /* Break alignment. */ \
+ "nopr %r7\n" \
+ ".globl " #name "_exrl\n" \
+ #name "_exrl:\n" \
+ ".cfi_startproc\n" \
+ "exrl %r0," #name "_2\n" \
+ "br %r14\n" \
+ ".cfi_endproc");
+
+/* noexec_1 is executable, noexec_2 is non-executable. */
+DEFINE_NX(noexec, 0xffa);
+
+/*
+ * noexec_cross_1 is executable, noexec_cross_2 crosses non-executable page
+ * boundary.
+ */
+DEFINE_NX(noexec_cross, 0xff8);
+
+/* noexec_full_1 and noexec_full_2 are non-executable. */
+DEFINE_NX(noexec_full, 0x322);
+
+int main(void)
+{
+ struct noexec_test noexec_tests[] = {
+ {
+ .name = "Fallthrough",
+ .func = noexec_1,
+ .page = noexec_2,
+ .expected_si_addr = noexec_2,
+ .arch = {
+ .expected_pswa = noexec_2,
+ .expected_r2 = 1,
+ },
+ },
+ {
+ .name = "Jump",
+ .func = noexec_2,
+ .page = noexec_2,
+ .expected_si_addr = noexec_2,
+ .arch = {
+ .expected_pswa = noexec_2,
+ .expected_r2 = 0,
+ },
+ },
+ {
+ .name = "EXRL",
+ .func = noexec_exrl,
+ .page = noexec_2,
+ .expected_si_addr = PAGE_ALIGN(noexec_end),
+ .arch = {
+ .expected_pswa = noexec_exrl,
+ .expected_r2 = 0,
+ },
+ },
+ {
+ .name = "Fallthrough [cross]",
+ .func = noexec_cross_1,
+ .page = PAGE_ALIGN(noexec_cross_end),
+ .expected_si_addr = PAGE_ALIGN(noexec_cross_end),
+ .arch = {
+ .expected_pswa = noexec_cross_2,
+ .expected_r2 = 1,
+ },
+ },
+ {
+ .name = "Jump [cross]",
+ .func = noexec_cross_2,
+ .page = PAGE_ALIGN(noexec_cross_end),
+ .expected_si_addr = PAGE_ALIGN(noexec_cross_end),
+ .arch = {
+ .expected_pswa = noexec_cross_2,
+ .expected_r2 = 0,
+ },
+ },
+ {
+ .name = "EXRL [cross]",
+ .func = noexec_cross_exrl,
+ .page = PAGE_ALIGN(noexec_cross_end),
+ .expected_si_addr = PAGE_ALIGN(noexec_cross_end),
+ .arch = {
+ .expected_pswa = noexec_cross_exrl,
+ .expected_r2 = 0,
+ },
+ },
+ {
+ .name = "Jump [full]",
+ .func = noexec_full_1,
+ .page = PAGE_ALIGN(noexec_full_1),
+ .expected_si_addr = PAGE_ALIGN(noexec_full_1),
+ .arch = {
+ .expected_pswa = noexec_full_1,
+ .expected_r2 = 0,
+ },
+ },
+ };
+
+ return test_noexec(noexec_tests,
+ sizeof(noexec_tests) / sizeof(noexec_tests[0]));
+}
diff --git a/tests/tcg/x86_64/Makefile.target b/tests/tcg/x86_64/Makefile.target
index b71a6bcd5e..c0e7e5b005 100644
--- a/tests/tcg/x86_64/Makefile.target
+++ b/tests/tcg/x86_64/Makefile.target
@@ -10,6 +10,7 @@ include $(SRC_PATH)/tests/tcg/i386/Makefile.target
ifeq ($(filter %-linux-user, $(TARGET)),$(TARGET))
X86_64_TESTS += vsyscall
+X86_64_TESTS += noexec
TESTS=$(MULTIARCH_TESTS) $(X86_64_TESTS) test-x86_64
else
TESTS=$(MULTIARCH_TESTS)
@@ -20,5 +21,5 @@ test-x86_64: LDFLAGS+=-lm -lc
test-x86_64: test-i386.c test-i386.h test-i386-shift.h test-i386-muldiv.h
$(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)
-vsyscall: $(SRC_PATH)/tests/tcg/x86_64/vsyscall.c
+%: $(SRC_PATH)/tests/tcg/x86_64/%.c
$(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)
diff --git a/tests/tcg/x86_64/noexec.c b/tests/tcg/x86_64/noexec.c
new file mode 100644
index 0000000000..ec07c9f0ba
--- /dev/null
+++ b/tests/tcg/x86_64/noexec.c
@@ -0,0 +1,116 @@
+#define _GNU_SOURCE
+
+struct arch_noexec_test {
+ void *expected_rip;
+ unsigned long expected_rdi;
+};
+
+#include "../multiarch/noexec.h"
+
+static void arch_check_mcontext(const struct arch_noexec_test *test,
+ const mcontext_t *ctx) {
+ if (ctx->gregs[REG_RIP] != (unsigned long)test->expected_rip) {
+ safe_puts("[ FAILED ] wrong rip");
+ _exit(1);
+ }
+
+ if (ctx->gregs[REG_RDI] != test->expected_rdi) {
+ safe_puts("[ FAILED ] wrong rdi");
+ _exit(1);
+ }
+}
+
+#define DEFINE_NX(name, offset) \
+ void name ## _1(int); \
+ void name ## _2(int); \
+ extern const short name ## _end[]; \
+ asm(/* Go to the specified page offset. */ \
+ ".align 0x1000\n" \
+ ".org .+" #offset "\n" \
+ /* %rdi is 0 on entry, overwrite it with 1. */ \
+ ".globl " #name "_1\n" \
+ #name "_1:\n" \
+ ".cfi_startproc\n" \
+ "movq $1,%rdi\n" \
+ /* Overwrite %rdi with 2. */ \
+ ".globl " #name "_2\n" \
+ #name "_2:\n" \
+ "movq $2,%rdi\n" \
+ "ret\n" \
+ /* End of code. */ \
+ ".globl " #name "_end\n" \
+ #name "_end:\n" \
+ ".cfi_endproc\n" \
+ /* Go to the next page. */ \
+ ".align 0x1000");
+
+/* noexec_1 is executable, noexec_2 is non-executable. */
+DEFINE_NX(noexec, 0xff9);
+
+/*
+ * noexec_cross_1 is executable, noexec_cross_2 crosses non-executable page
+ * boundary.
+ */
+DEFINE_NX(noexec_cross, 0xff8);
+
+/* noexec_full_1 and noexec_full_2 are non-executable. */
+DEFINE_NX(noexec_full, 0x321);
+
+int main(void)
+{
+ struct noexec_test noexec_tests[] = {
+ {
+ .name = "Fallthrough",
+ .func = noexec_1,
+ .page = noexec_2,
+ .expected_si_addr = noexec_2,
+ .arch = {
+ .expected_rip = noexec_2,
+ .expected_rdi = 1,
+ },
+ },
+ {
+ .name = "Jump",
+ .func = noexec_2,
+ .page = noexec_2,
+ .expected_si_addr = noexec_2,
+ .arch = {
+ .expected_rip = noexec_2,
+ .expected_rdi = 0,
+ },
+ },
+ {
+ .name = "Fallthrough [cross]",
+ .func = noexec_cross_1,
+ .page = PAGE_ALIGN(noexec_cross_end),
+ .expected_si_addr = PAGE_ALIGN(noexec_cross_end),
+ .arch = {
+ .expected_rip = noexec_cross_2,
+ .expected_rdi = 1,
+ },
+ },
+ {
+ .name = "Jump [cross]",
+ .func = noexec_cross_2,
+ .page = PAGE_ALIGN(noexec_cross_end),
+ .expected_si_addr = PAGE_ALIGN(noexec_cross_end),
+ .arch = {
+ .expected_rip = noexec_cross_2,
+ .expected_rdi = 0,
+ },
+ },
+ {
+ .name = "Jump [full]",
+ .func = noexec_full_1,
+ .page = PAGE_ALIGN(noexec_full_1),
+ .expected_si_addr = noexec_full_1,
+ .arch = {
+ .expected_rip = noexec_full_1,
+ .expected_rdi = 0,
+ },
+ },
+ };
+
+ return test_noexec(noexec_tests,
+ sizeof(noexec_tests) / sizeof(noexec_tests[0]));
+}
--
2.37.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v5 0/4] linux-user: Fix siginfo_t contents when jumping to non-readable pages
2022-08-17 15:05 [PATCH v5 0/4] linux-user: Fix siginfo_t contents when jumping to non-readable pages Ilya Leoshkevich
` (3 preceding siblings ...)
2022-08-17 15:05 ` [PATCH v5 4/4] tests/tcg: Test siginfo_t contents when jumping to non-readable pages Ilya Leoshkevich
@ 2022-08-17 16:23 ` Richard Henderson
2022-08-17 18:27 ` Ilya Leoshkevich
2022-08-18 16:55 ` Vivian Wang
5 siblings, 1 reply; 10+ messages in thread
From: Richard Henderson @ 2022-08-17 16:23 UTC (permalink / raw)
To: Ilya Leoshkevich, Laurent Vivier, Eduardo Habkost,
Marcel Apfelbaum, Philippe Mathieu-Daudé,
Yanan Wang, Paolo Bonzini, David Hildenbrand, Peter Maydell
Cc: qemu-devel, qemu-s390x, Christian Borntraeger
On 8/17/22 10:05, Ilya Leoshkevich wrote:
> Hi,
>
> I noticed that when we get a SEGV due to jumping to non-readable
> memory, sometimes si_addr and program counter in siginfo_t are slightly
> off. I tracked this down to the assumption that translators stop before
> the end of a page, while in reality they may stop right after it.
>
> Patch 1 fixes an invalidation issue, which may prevent SEGV from
> happening altogether.
> Patches 2-3 fix the main issue on x86_64 and s390x. Many other
> architectures have fixed-size instructions and are not affected.
> Patch 4 adds tests.
>
> Note: this series depends on [1].
Hah. I was just thinking that I should queue your patch set to tcg-next-7.2, and then
rebase my stuff off of that. It would ensure that I have your test cases in tree so that
I don't keep regressing them on you. :-)
I'll cherry pick the one patch you're depending on.
r~
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 0/4] linux-user: Fix siginfo_t contents when jumping to non-readable pages
2022-08-17 16:23 ` [PATCH v5 0/4] linux-user: Fix " Richard Henderson
@ 2022-08-17 18:27 ` Ilya Leoshkevich
0 siblings, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2022-08-17 18:27 UTC (permalink / raw)
To: Richard Henderson, Laurent Vivier, Eduardo Habkost,
Marcel Apfelbaum, Philippe Mathieu-Daudé,
Yanan Wang, Paolo Bonzini, David Hildenbrand, Peter Maydell
Cc: qemu-devel, qemu-s390x, Christian Borntraeger
On Wed, 2022-08-17 at 11:23 -0500, Richard Henderson wrote:
> On 8/17/22 10:05, Ilya Leoshkevich wrote:
> > Hi,
> >
> > I noticed that when we get a SEGV due to jumping to non-readable
> > memory, sometimes si_addr and program counter in siginfo_t are
> > slightly
> > off. I tracked this down to the assumption that translators stop
> > before
> > the end of a page, while in reality they may stop right after it.
> >
> > Patch 1 fixes an invalidation issue, which may prevent SEGV from
> > happening altogether.
> > Patches 2-3 fix the main issue on x86_64 and s390x. Many other
> > architectures have fixed-size instructions and are not affected.
> > Patch 4 adds tests.
> >
> > Note: this series depends on [1].
>
> Hah. I was just thinking that I should queue your patch set to tcg-
> next-7.2, and then
> rebase my stuff off of that. It would ensure that I have your test
> cases in tree so that
> I don't keep regressing them on you. :-)
>
> I'll cherry pick the one patch you're depending on.
>
>
> r~
I just checked and cherry-picking [1] and [2] before this series
should be enough.
[1] https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg02462.html
[2] https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg02461.html
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 0/4] linux-user: Fix siginfo_t contents when jumping to non-readable pages
2022-08-17 15:05 [PATCH v5 0/4] linux-user: Fix siginfo_t contents when jumping to non-readable pages Ilya Leoshkevich
` (4 preceding siblings ...)
2022-08-17 16:23 ` [PATCH v5 0/4] linux-user: Fix " Richard Henderson
@ 2022-08-18 16:55 ` Vivian Wang
2022-08-18 18:28 ` Ilya Leoshkevich
2022-08-18 18:32 ` Richard Henderson
5 siblings, 2 replies; 10+ messages in thread
From: Vivian Wang @ 2022-08-18 16:55 UTC (permalink / raw)
To: Ilya Leoshkevich, Laurent Vivier, Eduardo Habkost,
Marcel Apfelbaum, Philippe Mathieu-Daudé,
Yanan Wang, Richard Henderson, Paolo Bonzini, David Hildenbrand,
Peter Maydell
Cc: qemu-devel, qemu-s390x, Christian Borntraeger
[-- Attachment #1: Type: text/plain, Size: 3172 bytes --]
On 8/17/22 23:05, Ilya Leoshkevich wrote:
> Hi,
>
> I noticed that when we get a SEGV due to jumping to non-readable
> memory, sometimes si_addr and program counter in siginfo_t are slightly
> off. I tracked this down to the assumption that translators stop before
> the end of a page, while in reality they may stop right after it.
Hi,
Could this be related to issue 1155 [1]? On RISC-V, I'm getting
incorrect [m|s]tval/[m|s]epc combinations for page faults in system
emulation and incorrect si_addr and program counter on SIGSEGV in user
emulation. Since it seems to only affect instructions that cross page
boundaries, and RISC-V also has variable length instructions, it seems
that I've run into the same problem as what is fixed here.
Could this fix be extended be extended to targets/riscv?
dram
[1]: https://gitlab.com/qemu-project/qemu/-/issues/1155
> Patch 1 fixes an invalidation issue, which may prevent SEGV from
> happening altogether.
> Patches 2-3 fix the main issue on x86_64 and s390x. Many other
> architectures have fixed-size instructions and are not affected.
> Patch 4 adds tests.
>
> Note: this series depends on [1].
>
> Best regards,
> Ilya
>
> v1: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg00822.html
> v1 -> v2: Fix individual translators instead of translator_loop
> (Peter).
>
> v2: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg01079.html
> v2 -> v3: Peek at the next instruction on s390x (Richard).
> Undo more on i386 (Richard).
> Check PAGE_EXEC, not PAGE_READ (Peter, Richard).
>
> v3: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg01306.html
> v3 -> v4: Improve the commit message in patch 1 to better reflect what
> exactly is being fixed there.
> Factor out the is_same_page() patch (Richard).
> Do not touch the common code in the i386 fix (Richard).
>
> v4: https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg01747.html
> v4 -> v5: Drop patch 2.
> Use a different fix for the invalidation issue based on
> discussion with Richard [2].
>
> [1] https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg02472.html
> [2] https://lists.gnu.org/archive/html/qemu-devel/2022-08/msg02556.html
>
> Ilya Leoshkevich (4):
> linux-user: Clear tb_jmp_cache on mprotect()
> target/s390x: Make translator stop before the end of a page
> target/i386: Make translator stop before the end of a page
> tests/tcg: Test siginfo_t contents when jumping to non-readable pages
>
> linux-user/mmap.c | 14 +++
> target/i386/tcg/translate.c | 25 +++++-
> target/s390x/tcg/translate.c | 15 +++-
> tests/tcg/multiarch/noexec.h | 114 ++++++++++++++++++++++++
> tests/tcg/s390x/Makefile.target | 1 +
> tests/tcg/s390x/noexec.c | 145 +++++++++++++++++++++++++++++++
> tests/tcg/x86_64/Makefile.target | 3 +-
> tests/tcg/x86_64/noexec.c | 116 +++++++++++++++++++++++++
> 8 files changed, 427 insertions(+), 6 deletions(-)
> create mode 100644 tests/tcg/multiarch/noexec.h
> create mode 100644 tests/tcg/s390x/noexec.c
> create mode 100644 tests/tcg/x86_64/noexec.c
>
[-- Attachment #2: Type: text/html, Size: 4420 bytes --]
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 0/4] linux-user: Fix siginfo_t contents when jumping to non-readable pages
2022-08-18 16:55 ` Vivian Wang
@ 2022-08-18 18:28 ` Ilya Leoshkevich
2022-08-18 18:32 ` Richard Henderson
1 sibling, 0 replies; 10+ messages in thread
From: Ilya Leoshkevich @ 2022-08-18 18:28 UTC (permalink / raw)
To: Vivian Wang, Laurent Vivier, Eduardo Habkost, Marcel Apfelbaum,
Philippe Mathieu-Daudé,
Yanan Wang, Richard Henderson, Paolo Bonzini, David Hildenbrand,
Peter Maydell
Cc: qemu-devel, qemu-s390x, Christian Borntraeger
On Fri, 2022-08-19 at 00:55 +0800, Vivian Wang wrote:
> Hi,
> Could this be related to issue 1155 [1]? On RISC-V, I'm getting
> incorrect [m|s]tval/[m|s]epc combinations for page faults in system
> emulation and incorrect si_addr and program counter on SIGSEGV in
> user emulation. Since it seems to only affect instructions that cross
> page boundaries, and RISC-V also has variable length instructions, it
> seems that I've run into the same problem as what is fixed here.
> Could this fix be extended be extended to targets/riscv?
> dram
> [1]: https://gitlab.com/qemu-project/qemu/-/issues/1155
Yes, this looks quite similar.
I'm not too familiar with riscv, but I just googled [1].
If the following is correct:
---
However, the instruction set reserves enough opcode space to make it
possible to differentiate between 16-bit, 32-bit, 48-bit, and 64-bit
instructions. Instructions that start with binary 11 (in the lowest
bit position of the instruction) are 32-bit sized instructions (but one
pattern is reserved: so they cannot start with 11111). The compact
instructions use 00, 01, and 10 in that same position. 48-bit
instructions use starting sequence 011111, and 64-bit instructions
start with 0111111.
---
then we can fix this the same way s390x is being fixed here.
[1]
https://stackoverflow.com/questions/56874101/how-does-risc-v-variable-length-of-instruction-work-in-detail
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v5 0/4] linux-user: Fix siginfo_t contents when jumping to non-readable pages
2022-08-18 16:55 ` Vivian Wang
2022-08-18 18:28 ` Ilya Leoshkevich
@ 2022-08-18 18:32 ` Richard Henderson
1 sibling, 0 replies; 10+ messages in thread
From: Richard Henderson @ 2022-08-18 18:32 UTC (permalink / raw)
To: Vivian Wang, Ilya Leoshkevich, Laurent Vivier, Eduardo Habkost,
Marcel Apfelbaum, Philippe Mathieu-Daudé,
Yanan Wang, Paolo Bonzini, David Hildenbrand, Peter Maydell
Cc: qemu-devel, qemu-s390x, Christian Borntraeger
On 8/18/22 09:55, Vivian Wang wrote:
> On 8/17/22 23:05, Ilya Leoshkevich wrote:
>> Hi,
>>
>> I noticed that when we get a SEGV due to jumping to non-readable
>> memory, sometimes si_addr and program counter in siginfo_t are slightly
>> off. I tracked this down to the assumption that translators stop before
>> the end of a page, while in reality they may stop right after it.
>
> Hi,
>
> Could this be related to issue 1155 [1]? On RISC-V, I'm getting incorrect
> [m|s]tval/[m|s]epc combinations for page faults in system emulation and incorrect si_addr
> and program counter on SIGSEGV in user emulation. Since it seems to only affect
> instructions that cross page boundaries, and RISC-V also has variable length instructions,
> it seems that I've run into the same problem as what is fixed here.
It seems likely, and the code at the end of riscv_tr_translate_insn is wrong.
> Could this fix be extended be extended to targets/riscv?
I'll write up something.
r~
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2022-08-18 18:39 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-08-17 15:05 [PATCH v5 0/4] linux-user: Fix siginfo_t contents when jumping to non-readable pages Ilya Leoshkevich
2022-08-17 15:05 ` [PATCH v5 1/4] linux-user: Clear tb_jmp_cache on mprotect() Ilya Leoshkevich
2022-08-17 15:05 ` [PATCH v5 2/4] target/s390x: Make translator stop before the end of a page Ilya Leoshkevich
2022-08-17 15:05 ` [PATCH v5 3/4] target/i386: " Ilya Leoshkevich
2022-08-17 15:05 ` [PATCH v5 4/4] tests/tcg: Test siginfo_t contents when jumping to non-readable pages Ilya Leoshkevich
2022-08-17 16:23 ` [PATCH v5 0/4] linux-user: Fix " Richard Henderson
2022-08-17 18:27 ` Ilya Leoshkevich
2022-08-18 16:55 ` Vivian Wang
2022-08-18 18:28 ` Ilya Leoshkevich
2022-08-18 18:32 ` Richard Henderson
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.