* [kvm-unit-tests PATCH] x86/emulator64: Extend non-canonical memory access tests with CR2 coverage
@ 2025-06-12 14:16 Mathias Krause
2025-06-26 14:49 ` Sean Christopherson
0 siblings, 1 reply; 4+ messages in thread
From: Mathias Krause @ 2025-06-12 14:16 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini; +Cc: kvm, Mathias Krause
Extend the non-canonical memory access tests to verify CR2 stays
unchanged.
There's currently a bug in QEMU/TCG that breaks that assumption.
Link: https://gitlab.com/qemu-project/qemu/-/issues/928
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
---
x86/emulator64.c | 26 ++++++++++++++++++++++++++
1 file changed, 26 insertions(+)
diff --git a/x86/emulator64.c b/x86/emulator64.c
index 5d1bb0f06d4f..abef2bda29f1 100644
--- a/x86/emulator64.c
+++ b/x86/emulator64.c
@@ -325,16 +325,39 @@ static void test_mmx_movq_mf(uint64_t *mem)
report(exception_vector() == MF_VECTOR, "movq mmx generates #MF");
}
+#define CR2_REF_VALUE 0xdecafbadUL
+
+static void setup_cr2(void)
+{
+ write_cr2(CR2_REF_VALUE);
+}
+
+static void check_cr2(void)
+{
+ unsigned long cr2 = read_cr2();
+
+ if (cr2 == CR2_REF_VALUE) {
+ report(true, "CR2 unchanged");
+ } else {
+ report(false, "CR2 changed from %#lx to %#lx", CR2_REF_VALUE, cr2);
+ setup_cr2();
+ }
+}
+
static void test_jmp_noncanonical(uint64_t *mem)
{
+ setup_cr2();
*mem = NONCANONICAL;
asm volatile (ASM_TRY("1f") "jmp *%0; 1:" : : "m"(*mem));
report(exception_vector() == GP_VECTOR,
"jump to non-canonical address");
+ check_cr2();
}
static void test_reg_noncanonical(void)
{
+ setup_cr2();
+
/* RAX based, should #GP(0) */
asm volatile(ASM_TRY("1f") "orq $0, (%[noncanonical]); 1:"
: : [noncanonical]"a"(NONCANONICAL));
@@ -342,6 +365,7 @@ static void test_reg_noncanonical(void)
"non-canonical memory access, should %s(0), got %s(%u)",
exception_mnemonic(GP_VECTOR),
exception_mnemonic(exception_vector()), exception_error_code());
+ check_cr2();
/* RSP based, should #SS(0) */
asm volatile(ASM_TRY("1f") "orq $0, (%%rsp,%[noncanonical],1); 1:"
@@ -350,6 +374,7 @@ static void test_reg_noncanonical(void)
"non-canonical rsp-based access, should %s(0), got %s(%u)",
exception_mnemonic(SS_VECTOR),
exception_mnemonic(exception_vector()), exception_error_code());
+ check_cr2();
/* RBP based, should #SS(0) */
asm volatile(ASM_TRY("1f") "orq $0, (%%rbp,%[noncanonical],1); 1:"
@@ -358,6 +383,7 @@ static void test_reg_noncanonical(void)
"non-canonical rbp-based access, should %s(0), got %s(%u)",
exception_mnemonic(SS_VECTOR),
exception_mnemonic(exception_vector()), exception_error_code());
+ check_cr2();
}
static void test_movabs(uint64_t *mem)
--
2.30.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [kvm-unit-tests PATCH] x86/emulator64: Extend non-canonical memory access tests with CR2 coverage
2025-06-12 14:16 [kvm-unit-tests PATCH] x86/emulator64: Extend non-canonical memory access tests with CR2 coverage Mathias Krause
@ 2025-06-26 14:49 ` Sean Christopherson
2025-06-27 14:33 ` Mathias Krause
2025-11-18 22:39 ` Sean Christopherson
0 siblings, 2 replies; 4+ messages in thread
From: Sean Christopherson @ 2025-06-26 14:49 UTC (permalink / raw)
To: Mathias Krause; +Cc: Paolo Bonzini, kvm
[-- Attachment #1: Type: text/plain, Size: 3659 bytes --]
On Thu, Jun 12, 2025, Mathias Krause wrote:
> Extend the non-canonical memory access tests to verify CR2 stays
> unchanged.
>
> There's currently a bug in QEMU/TCG that breaks that assumption.
>
> Link: https://gitlab.com/qemu-project/qemu/-/issues/928
> Signed-off-by: Mathias Krause <minipli@grsecurity.net>
> ---
> x86/emulator64.c | 26 ++++++++++++++++++++++++++
> 1 file changed, 26 insertions(+)
>
> diff --git a/x86/emulator64.c b/x86/emulator64.c
> index 5d1bb0f06d4f..abef2bda29f1 100644
> --- a/x86/emulator64.c
> +++ b/x86/emulator64.c
> @@ -325,16 +325,39 @@ static void test_mmx_movq_mf(uint64_t *mem)
> report(exception_vector() == MF_VECTOR, "movq mmx generates #MF");
> }
>
> +#define CR2_REF_VALUE 0xdecafbadUL
> +
> +static void setup_cr2(void)
> +{
> + write_cr2(CR2_REF_VALUE);
> +}
> +
> +static void check_cr2(void)
> +{
> + unsigned long cr2 = read_cr2();
> +
> + if (cr2 == CR2_REF_VALUE) {
> + report(true, "CR2 unchanged");
> + } else {
> + report(false, "CR2 changed from %#lx to %#lx", CR2_REF_VALUE, cr2);
> + setup_cr2();
Writing CR2 isn't expensive in the grand scheme, so rather than conditionally
re-write CR2, I think it makes sense to write CR2 at the start of every testcase,
and then just do "report(cr2 == CR2_REF_VALUE".
> + }
> +}
> +
> static void test_jmp_noncanonical(uint64_t *mem)
> {
> + setup_cr2();
> *mem = NONCANONICAL;
> asm volatile (ASM_TRY("1f") "jmp *%0; 1:" : : "m"(*mem));
> report(exception_vector() == GP_VECTOR,
> "jump to non-canonical address");
> + check_cr2();
> }
>
> static void test_reg_noncanonical(void)
> {
> + setup_cr2();
> +
> /* RAX based, should #GP(0) */
> asm volatile(ASM_TRY("1f") "orq $0, (%[noncanonical]); 1:"
> : : [noncanonical]"a"(NONCANONICAL));
> @@ -342,6 +365,7 @@ static void test_reg_noncanonical(void)
> "non-canonical memory access, should %s(0), got %s(%u)",
> exception_mnemonic(GP_VECTOR),
> exception_mnemonic(exception_vector()), exception_error_code());
> + check_cr2();
And then rather than add more copy+paste, what if we add a macro to handle the
checks? Then the CR2 validation can slot in nicely (and maybe someday the macro
could be used outside of the x86/emulator64.c).
Attached patches yield:
#define CR2_REF_VALUE 0xdecafbadUL
#define ASM_TRY_NONCANONICAL(insn, inputs, access, ex_vector) \
do { \
unsigned int vector, ec; \
\
write_cr2(CR2_REF_VALUE); \
\
asm volatile(ASM_TRY("1f") insn "; 1:" :: inputs); \
\
vector = exception_vector(); \
ec = exception_error_code(); \
\
report(vector == ex_vector && !ec, \
"non-canonical " access ", should %s(0), got %s(%u)", \
exception_mnemonic(ex_vector), exception_mnemonic(vector), ec); \
\
if (vector != PF_VECTOR) { \
unsigned long cr2 = read_cr2(); \
\
report(cr2 == CR2_REF_VALUE, \
"Wanted CR2 '0x%lx', got '0x%lx", CR2_REF_VALUE, cr2); \
} \
} while (0)
static void test_jmp_noncanonical(uint64_t *mem)
{
*mem = NONCANONICAL;
ASM_TRY_NONCANONICAL("jmp *%0", "m"(*mem), "jmp", GP_VECTOR);
}
static void test_reg_noncanonical(void)
{
/* RAX based, should #GP(0) */
ASM_TRY_NONCANONICAL("orq $0, (%[nc])", [nc]"a"(NONCANONICAL),
"memory access", GP_VECTOR);
/* RSP based, should #SS(0) */
ASM_TRY_NONCANONICAL("orq $0, (%%rsp,%[nc],1)", [nc]"r"(NONCANONICAL),
"rsp-based access", SS_VECTOR);
/* RBP based, should #SS(0) */
ASM_TRY_NONCANONICAL("orq $0, (%%rbp,%[nc],1)", [nc]"r"(NONCANONICAL),
"rbp-based access", SS_VECTOR);
}
[-- Attachment #2: 0001-x86-emulator64-Add-macro-to-test-emulation-of-non-ca.patch --]
[-- Type: text/x-diff, Size: 3492 bytes --]
From 0a7ee3543ef899ea36614ddff56c306dd63c341c Mon Sep 17 00:00:00 2001
From: Sean Christopherson <seanjc@google.com>
Date: Thu, 26 Jun 2025 07:39:43 -0700
Subject: [PATCH 1/2] x86/emulator64: Add macro to test emulation of
non-canonical accesses
Add a macro to "try" and check a non-canonical access. In addition to
de-duplicating the checking logic, this will allow extending the logic to
verify that CR2 isn't incorrectly modified, e.g. on #GP/#SS.
No functional change intended (ignoring the newly added check on the error
code for the JMP case).
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
x86/emulator64.c | 43 ++++++++++++++++++++++---------------------
1 file changed, 22 insertions(+), 21 deletions(-)
diff --git a/x86/emulator64.c b/x86/emulator64.c
index 138903af..21df3b0a 100644
--- a/x86/emulator64.c
+++ b/x86/emulator64.c
@@ -325,39 +325,40 @@ static void test_mmx_movq_mf(uint64_t *mem)
report(exception_vector() == MF_VECTOR, "movq mmx generates #MF");
}
+#define ASM_TRY_NONCANONICAL(insn, inputs, access, ex_vector) \
+do { \
+ unsigned int vector, ec; \
+ \
+ asm volatile(ASM_TRY("1f") insn "; 1:" :: inputs); \
+ \
+ vector = exception_vector(); \
+ ec = exception_error_code(); \
+ \
+ report(vector == ex_vector && !ec, \
+ "non-canonical " access ", should %s(0), got %s(%u)", \
+ exception_mnemonic(ex_vector), exception_mnemonic(vector), ec); \
+} while (0)
+
static void test_jmp_noncanonical(uint64_t *mem)
{
*mem = NONCANONICAL;
- asm volatile (ASM_TRY("1f") "jmp *%0; 1:" : : "m"(*mem));
- report(exception_vector() == GP_VECTOR,
- "jump to non-canonical address");
+
+ ASM_TRY_NONCANONICAL("jmp *%0", "m"(*mem), "jmp", GP_VECTOR);
}
static void test_reg_noncanonical(void)
{
/* RAX based, should #GP(0) */
- asm volatile(ASM_TRY("1f") "orq $0, (%[noncanonical]); 1:"
- : : [noncanonical]"a"(NONCANONICAL));
- report(exception_vector() == GP_VECTOR && exception_error_code() == 0,
- "non-canonical memory access, should %s(0), got %s(%u)",
- exception_mnemonic(GP_VECTOR),
- exception_mnemonic(exception_vector()), exception_error_code());
+ ASM_TRY_NONCANONICAL("orq $0, (%[nc])", [nc]"a"(NONCANONICAL),
+ "memory access", GP_VECTOR);
/* RSP based, should #SS(0) */
- asm volatile(ASM_TRY("1f") "orq $0, (%%rsp,%[noncanonical],1); 1:"
- : : [noncanonical]"r"(NONCANONICAL));
- report(exception_vector() == SS_VECTOR && exception_error_code() == 0,
- "non-canonical rsp-based access, should %s(0), got %s(%u)",
- exception_mnemonic(SS_VECTOR),
- exception_mnemonic(exception_vector()), exception_error_code());
+ ASM_TRY_NONCANONICAL("orq $0, (%%rsp,%[nc],1)", [nc]"r"(NONCANONICAL),
+ "rsp-based access", SS_VECTOR);
/* RBP based, should #SS(0) */
- asm volatile(ASM_TRY("1f") "orq $0, (%%rbp,%[noncanonical],1); 1:"
- : : [noncanonical]"r"(NONCANONICAL));
- report(exception_vector() == SS_VECTOR && exception_error_code() == 0,
- "non-canonical rbp-based access, should %s(0), got %s(%u)",
- exception_mnemonic(SS_VECTOR),
- exception_mnemonic(exception_vector()), exception_error_code());
+ ASM_TRY_NONCANONICAL("orq $0, (%%rbp,%[nc],1)", [nc]"r"(NONCANONICAL),
+ "rbp-based access", SS_VECTOR);
}
static void test_movabs(uint64_t *mem)
base-commit: 525bdb5d65d51a367341f471eb1bcd505d73c51f
--
2.50.0.727.gbf7dc18ff4-goog
[-- Attachment #3: 0002-x86-emulator64-Extend-non-canonical-memory-access-te.patch --]
[-- Type: text/x-diff, Size: 1767 bytes --]
From efb11a007a0e041ef029753b0b98abe071008334 Mon Sep 17 00:00:00 2001
From: Mathias Krause <minipli@grsecurity.net>
Date: Thu, 26 Jun 2025 07:42:51 -0700
Subject: [PATCH 2/2] x86/emulator64: Extend non-canonical memory access tests
with CR2 coverage
Extend the non-canonical memory access tests to verify CR2 stays
unchanged.
There's currently a bug in QEMU/TCG that breaks that assumption.
Link: https://gitlab.com/qemu-project/qemu/-/issues/928
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
Signed-off-by: Sean Christopherson <seanjc@google.com>
---
x86/emulator64.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/x86/emulator64.c b/x86/emulator64.c
index 21df3b0a..6a85122f 100644
--- a/x86/emulator64.c
+++ b/x86/emulator64.c
@@ -325,10 +325,14 @@ static void test_mmx_movq_mf(uint64_t *mem)
report(exception_vector() == MF_VECTOR, "movq mmx generates #MF");
}
+#define CR2_REF_VALUE 0xdecafbadUL
+
#define ASM_TRY_NONCANONICAL(insn, inputs, access, ex_vector) \
do { \
unsigned int vector, ec; \
\
+ write_cr2(CR2_REF_VALUE); \
+ \
asm volatile(ASM_TRY("1f") insn "; 1:" :: inputs); \
\
vector = exception_vector(); \
@@ -337,6 +341,13 @@ do { \
report(vector == ex_vector && !ec, \
"non-canonical " access ", should %s(0), got %s(%u)", \
exception_mnemonic(ex_vector), exception_mnemonic(vector), ec); \
+ \
+ if (vector != PF_VECTOR) { \
+ unsigned long cr2 = read_cr2(); \
+ \
+ report(cr2 == CR2_REF_VALUE, \
+ "Wanted CR2 '0x%lx', got '0x%lx", CR2_REF_VALUE, cr2); \
+ } \
} while (0)
static void test_jmp_noncanonical(uint64_t *mem)
--
2.50.0.727.gbf7dc18ff4-goog
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [kvm-unit-tests PATCH] x86/emulator64: Extend non-canonical memory access tests with CR2 coverage
2025-06-26 14:49 ` Sean Christopherson
@ 2025-06-27 14:33 ` Mathias Krause
2025-11-18 22:39 ` Sean Christopherson
1 sibling, 0 replies; 4+ messages in thread
From: Mathias Krause @ 2025-06-27 14:33 UTC (permalink / raw)
To: Sean Christopherson; +Cc: Paolo Bonzini, kvm
On 26.06.25 16:49, Sean Christopherson wrote:
> On Thu, Jun 12, 2025, Mathias Krause wrote:
>> Extend the non-canonical memory access tests to verify CR2 stays
>> unchanged.
>>
>> There's currently a bug in QEMU/TCG that breaks that assumption.
>>
>> Link: https://gitlab.com/qemu-project/qemu/-/issues/928
>> Signed-off-by: Mathias Krause <minipli@grsecurity.net>
>> ---
>> x86/emulator64.c | 26 ++++++++++++++++++++++++++
>> 1 file changed, 26 insertions(+)
>>
>> diff --git a/x86/emulator64.c b/x86/emulator64.c
>> index 5d1bb0f06d4f..abef2bda29f1 100644
>> --- a/x86/emulator64.c
>> +++ b/x86/emulator64.c
>> @@ -325,16 +325,39 @@ static void test_mmx_movq_mf(uint64_t *mem)
>> report(exception_vector() == MF_VECTOR, "movq mmx generates #MF");
>> }
>>
>> +#define CR2_REF_VALUE 0xdecafbadUL
>> +
>> +static void setup_cr2(void)
>> +{
>> + write_cr2(CR2_REF_VALUE);
>> +}
>> +
>> +static void check_cr2(void)
>> +{
>> + unsigned long cr2 = read_cr2();
>> +
>> + if (cr2 == CR2_REF_VALUE) {
>> + report(true, "CR2 unchanged");
>> + } else {
>> + report(false, "CR2 changed from %#lx to %#lx", CR2_REF_VALUE, cr2);
>> + setup_cr2();
>
> Writing CR2 isn't expensive in the grand scheme, so rather than conditionally
> re-write CR2, I think it makes sense to write CR2 at the start of every testcase,
> and then just do "report(cr2 == CR2_REF_VALUE".
Probably makes sense.
>
>> + }
>> +}
>> +
>> static void test_jmp_noncanonical(uint64_t *mem)
>> {
>> + setup_cr2();
>> *mem = NONCANONICAL;
>> asm volatile (ASM_TRY("1f") "jmp *%0; 1:" : : "m"(*mem));
>> report(exception_vector() == GP_VECTOR,
>> "jump to non-canonical address");
>> + check_cr2();
>> }
>>
>> static void test_reg_noncanonical(void)
>> {
>> + setup_cr2();
>> +
>> /* RAX based, should #GP(0) */
>> asm volatile(ASM_TRY("1f") "orq $0, (%[noncanonical]); 1:"
>> : : [noncanonical]"a"(NONCANONICAL));
>> @@ -342,6 +365,7 @@ static void test_reg_noncanonical(void)
>> "non-canonical memory access, should %s(0), got %s(%u)",
>> exception_mnemonic(GP_VECTOR),
>> exception_mnemonic(exception_vector()), exception_error_code());
>> + check_cr2();
>
> And then rather than add more copy+paste, what if we add a macro to handle the
> checks? Then the CR2 validation can slot in nicely (and maybe someday the macro
> could be used outside of the x86/emulator64.c).
>
> Attached patches yield:
>
> #define CR2_REF_VALUE 0xdecafbadUL
>
> #define ASM_TRY_NONCANONICAL(insn, inputs, access, ex_vector) \
> do { \
> unsigned int vector, ec; \
> \
> write_cr2(CR2_REF_VALUE); \
> \
> asm volatile(ASM_TRY("1f") insn "; 1:" :: inputs); \
> \
> vector = exception_vector(); \
> ec = exception_error_code(); \
> \
> report(vector == ex_vector && !ec, \
> "non-canonical " access ", should %s(0), got %s(%u)", \
> exception_mnemonic(ex_vector), exception_mnemonic(vector), ec); \
> \
> if (vector != PF_VECTOR) { \
> unsigned long cr2 = read_cr2(); \
> \
> report(cr2 == CR2_REF_VALUE, \
> "Wanted CR2 '0x%lx', got '0x%lx", CR2_REF_VALUE, cr2); \
> } \
> } while (0)
>
> static void test_jmp_noncanonical(uint64_t *mem)
> {
> *mem = NONCANONICAL;
>
> ASM_TRY_NONCANONICAL("jmp *%0", "m"(*mem), "jmp", GP_VECTOR);
> }
>
> static void test_reg_noncanonical(void)
> {
> /* RAX based, should #GP(0) */
> ASM_TRY_NONCANONICAL("orq $0, (%[nc])", [nc]"a"(NONCANONICAL),
> "memory access", GP_VECTOR);
>
> /* RSP based, should #SS(0) */
> ASM_TRY_NONCANONICAL("orq $0, (%%rsp,%[nc],1)", [nc]"r"(NONCANONICAL),
> "rsp-based access", SS_VECTOR);
>
> /* RBP based, should #SS(0) */
> ASM_TRY_NONCANONICAL("orq $0, (%%rbp,%[nc],1)", [nc]"r"(NONCANONICAL),
> "rbp-based access", SS_VECTOR);
> }
Yeah, looks much nicer indeed!
Both patches:
Reviewed-by: Mathias Krause <minipli@grsecurity.net>
Tested-by: Mathias Krause <minipli@grsecurity.net>
Thanks a lot,
Mathias
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [kvm-unit-tests PATCH] x86/emulator64: Extend non-canonical memory access tests with CR2 coverage
2025-06-26 14:49 ` Sean Christopherson
2025-06-27 14:33 ` Mathias Krause
@ 2025-11-18 22:39 ` Sean Christopherson
1 sibling, 0 replies; 4+ messages in thread
From: Sean Christopherson @ 2025-11-18 22:39 UTC (permalink / raw)
To: Mathias Krause; +Cc: Paolo Bonzini, kvm
On Thu, Jun 26, 2025, Sean Christopherson wrote:
> On Thu, Jun 12, 2025, Mathias Krause wrote:
> And then rather than add more copy+paste, what if we add a macro to handle the
> checks? Then the CR2 validation can slot in nicely (and maybe someday the macro
> could be used outside of the x86/emulator64.c).
>
> Attached patches yield:
Applied said patches to kvm-x86 next.
[1/2] x86/emulator64: Add macro to test emulation of non-canonical accesses
https://github.com/kvm-x86/kvm-unit-tests/commit/9f5f06b1
[2/2] x86/emulator64: Extend non-canonical memory access tests with CR2 coverage
https://github.com/kvm-x86/kvm-unit-tests/commit/a55d5602
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-11-18 22:39 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-12 14:16 [kvm-unit-tests PATCH] x86/emulator64: Extend non-canonical memory access tests with CR2 coverage Mathias Krause
2025-06-26 14:49 ` Sean Christopherson
2025-06-27 14:33 ` Mathias Krause
2025-11-18 22:39 ` Sean Christopherson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox