diff mbox series

[kvm-unit-tests,2/2] x86/access: CR0.WP toggling write access test

Message ID 20230327181911.51655-3-minipli@grsecurity.net (mailing list archive)
State New, archived
Headers show
Series Test for CR0.WP=0/1 r/o write access | expand

Commit Message

Mathias Krause March 27, 2023, 6:19 p.m. UTC 
We already have tests that verify a write access to an r/o page is
successful when CR0.WP=0, but we lack a test that explicitly verifies
that the same access will fail after we set CR0.WP=1 without flushing
any associated TLB entries either explicitly (INVLPG) or implicitly
(write to CR3). Add such a test.

Signed-off-by: Mathias Krause <minipli@grsecurity.net>
---
 x86/access.c | 46 ++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 44 insertions(+), 2 deletions(-)

Comments

Mathias Krause March 31, 2023, 10:03 a.m. UTC | #1 
On 27.03.23 20:19, Mathias Krause wrote:
> We already have tests that verify a write access to an r/o page is
> successful when CR0.WP=0, but we lack a test that explicitly verifies
> that the same access will fail after we set CR0.WP=1 without flushing
> any associated TLB entries either explicitly (INVLPG) or implicitly
> (write to CR3). Add such a test.

This test is insufficient to test all corner cases, as noted in [1].
Especially the emulator needs to be tested as well.

I'll send a v2 soon!

Mathias
Mathias Krause March 31, 2023, 10:05 a.m. UTC | #2 
On 31.03.23 12:03, Mathias Krause wrote:
> This test is insufficient to test all corner cases, as noted in [1].
> [...]

Forgot the link:

[1]
https://lore.kernel.org/kvm/ea3a8fbc-2bf8-7442-e498-3e5818384c83@grsecurity.net/
diff mbox series

Patch

diff --git a/x86/access.c b/x86/access.c
index 203353a3f74f..5fe2a89ddfa9 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -575,9 +575,10 @@  fault:
 		at->expected_error &= ~PFERR_FETCH_MASK;
 }
 
-static void ac_set_expected_status(ac_test_t *at)
+static void __ac_set_expected_status(ac_test_t *at, bool flush)
 {
-	invlpg(at->virt);
+	if (flush)
+		invlpg(at->virt);
 
 	if (at->ptep)
 		at->expected_pte = *at->ptep;
@@ -599,6 +600,11 @@  static void ac_set_expected_status(ac_test_t *at)
 	ac_emulate_access(at, at->flags);
 }
 
+static void ac_set_expected_status(ac_test_t *at)
+{
+	__ac_set_expected_status(at, true);
+}
+
 static pt_element_t ac_get_pt(ac_test_t *at, int i, pt_element_t *ptep)
 {
 	pt_element_t pte;
@@ -1061,6 +1067,41 @@  err:
 	return 0;
 }
 
+static int check_write_cr0wp(ac_pt_env_t *pt_env)
+{
+	ac_test_t at1;
+
+	ac_test_init(&at1, 0xffff923042007000ul, pt_env);
+
+	at1.flags = AC_PDE_PRESENT_MASK | AC_PTE_PRESENT_MASK |
+		    AC_PDE_ACCESSED_MASK | AC_PTE_ACCESSED_MASK |
+		    AC_CPU_CR0_WP_MASK |
+		    AC_ACCESS_WRITE_MASK;
+	ac_test_setup_ptes(&at1);
+
+	/*
+	 * Write to r/o page with cr0.wp=0, then try again
+	 * with cr0.wp=1 and expect a page fault to happen.
+	 */
+	if (!ac_test_do_access(&at1)) {
+		printf("%s: CR0.WP=0 r/o write fail\n", __FUNCTION__);
+		goto err;
+	}
+
+	at1.flags &= ~AC_CPU_CR0_WP_MASK;
+	__ac_set_expected_status(&at1, false);
+
+	if (!ac_test_do_access(&at1)) {
+		printf("%s: CR0.WP=1 r/o write deny fail\n", __FUNCTION__);
+		goto err;
+	}
+
+	return 1;
+
+err:
+	return 0;
+}
+
 static int check_effective_sp_permissions(ac_pt_env_t *pt_env)
 {
 	unsigned long ptr1 = 0xffff923480000000;
@@ -1150,6 +1191,7 @@  const ac_test_fn ac_test_cases[] =
 	check_pfec_on_prefetch_pte,
 	check_large_pte_dirty_for_nowp,
 	check_smep_andnot_wp,
+	check_write_cr0wp,
 	check_effective_sp_permissions,
 };