diff mbox series

[kvm-unit-tests,v2,4/4] x86/access: Try emulation for CR0.WP test as well

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

Commit Message

Mathias Krause March 31, 2023, 1:57 p.m. UTC 
Enhance the CR.WP toggling test to do additional tests via the emulator
as these used to trigger bugs when CR0.WP is guest owned.

Link: https://lore.kernel.org/kvm/ea3a8fbc-2bf8-7442-e498-3e5818384c83@grsecurity.net/
Signed-off-by: Mathias Krause <minipli@grsecurity.net>
---
 x86/access.c | 46 +++++++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 15 deletions(-)

Comments

Sean Christopherson March 31, 2023, 4:24 p.m. UTC | #1 
On Fri, Mar 31, 2023, Mathias Krause wrote:
> Enhance the CR.WP toggling test to do additional tests via the emulator
> as these used to trigger bugs when CR0.WP is guest owned.
> 
> Link: https://lore.kernel.org/kvm/ea3a8fbc-2bf8-7442-e498-3e5818384c83@grsecurity.net/
> Signed-off-by: Mathias Krause <minipli@grsecurity.net>
> ---
>  x86/access.c | 46 +++++++++++++++++++++++++++++++---------------
>  1 file changed, 31 insertions(+), 15 deletions(-)
> 
> diff --git a/x86/access.c b/x86/access.c
> index ae5e7d8e8892..21967434bc18 100644
> --- a/x86/access.c
> +++ b/x86/access.c
> @@ -1107,27 +1107,43 @@ static int check_write_cr0wp(ac_pt_env_t *pt_env)
>  	 * We load CR0.WP with the inverse value of what would be used during
>  	 * the access test and toggle EFER.NX to flush and rebuild the current
>  	 * MMU context based on that value.
> +	 *
> +	 * This used to trigger a bug in the emulator we try to test via FEP.
>  	 */
> +	for (;;) {

As suggested in patch 2, I'm pretty sure it's easier to use a helper, and have
the print statement react to at->flags.
diff mbox series

Patch

diff --git a/x86/access.c b/x86/access.c
index ae5e7d8e8892..21967434bc18 100644
--- a/x86/access.c
+++ b/x86/access.c
@@ -1107,27 +1107,43 @@  static int check_write_cr0wp(ac_pt_env_t *pt_env)
 	 * We load CR0.WP with the inverse value of what would be used during
 	 * the access test and toggle EFER.NX to flush and rebuild the current
 	 * MMU context based on that value.
+	 *
+	 * This used to trigger a bug in the emulator we try to test via FEP.
 	 */
+	for (;;) {
+		const char *fep = (at.flags & AC_FEP_MASK) ? "FEP " : "";
 
-	set_cr0_wp(1);
-	set_efer_nx(1);
-	set_efer_nx(0);
+		set_cr0_wp(1);
+		set_efer_nx(1);
+		set_efer_nx(0);
 
-	if (!ac_test_do_access(&at)) {
-		printf("%s: CR0.WP=0 r/o write fail\n", __FUNCTION__);
-		err++;
-	}
+		if (!ac_test_do_access(&at)) {
+			printf("%s: %sCR0.WP=0 r/o write fail\n", __FUNCTION__, fep);
+			err++;
+		}
 
-	at.flags |= AC_CPU_CR0_WP_MASK;
-	__ac_set_expected_status(&at, false);
+		at.flags |= AC_CPU_CR0_WP_MASK;
+		__ac_set_expected_status(&at, false);
 
-	set_cr0_wp(0);
-	set_efer_nx(1);
-	set_efer_nx(0);
+		set_cr0_wp(0);
+		set_efer_nx(1);
+		set_efer_nx(0);
 
-	if (!ac_test_do_access(&at)) {
-		printf("%s: CR0.WP=1 r/o write deny fail\n", __FUNCTION__);
-		err++;
+		if (!ac_test_do_access(&at)) {
+			printf("%s: %sCR0.WP=1 r/o write deny fail\n", __FUNCTION__, fep);
+			err++;
+		}
+
+		if (!fep_available)
+			break;
+
+		if (at.flags & AC_FEP_MASK)
+			break;
+
+		/* Re-test via the emulator */
+		at.flags |= AC_FEP_MASK;
+		at.flags ^= AC_CPU_CR0_WP_MASK;
+		__ac_set_expected_status(&at, false);
 	}
 
 	return err == 0;