From bdcea2bcb07c71b32fee317c9d6b01570dbeb59f Mon Sep 17 00:00:00 2001 From: Paolo Bonzini Date: Fri, 23 Feb 2018 13:23:29 -0500 Subject: [PATCH] target-i386: check for PKU even for non-writable pages Xiao Guangrong ran kvm-unit-tests on an actual machine with PKU and found that it fails: test pte.p pte.user pde.p pde.user pde.a pde.pse pkru.wd pkey=1 user write efer.nx cr4.pke: FAIL: error code 27 expected 7 Dump mapping: address: 0x123400000000 ------L4: 2ebe007 ------L3: 2ebf007 ------L2: 8000000020000a5 (All failures are combinations of "pde.user pde.p pkru.wd pkey=1", plus either "pde.pse" or "pte.p pte.user", plus one of "user cr0.wp", "cr0.wp" or "user", plus unimportant bits such as accessed/dirty or efer.nx). So PFEC.PKEY is set even if the ordinary check failed (which it did because pde.w is zero). Adjust QEMU to match behavior of silicon. Backports commit 44d066a2f770ee9d61fd1c2a609bdf2a994dfdf7 from qemu --- qemu/target-i386/helper.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/qemu/target-i386/helper.c b/qemu/target-i386/helper.c index 2b15c6a1..3692b3a7 100644 --- a/qemu/target-i386/helper.c +++ b/qemu/target-i386/helper.c @@ -731,28 +731,32 @@ do_check_protect_pse36: !((env->cr[4] & CR4_SMEP_MASK) && (ptep & PG_USER_MASK)))) { prot |= PAGE_EXEC; } - if ((prot & (1 << is_write1)) == 0) { - goto do_fault_protect; - } if ((env->cr[4] & CR4_PKE_MASK) && (env->hflags & HF_LMA_MASK) && (ptep & PG_USER_MASK) && env->pkru) { uint32_t pk = (pte & PG_PKRU_MASK) >> PG_PKRU_BIT; uint32_t pkru_ad = (env->pkru >> pk * 2) & 1; uint32_t pkru_wd = (env->pkru >> pk * 2) & 2; + uint32_t pkru_prot = PAGE_READ | PAGE_WRITE | PAGE_EXEC; if (pkru_ad) { - prot &= ~(PAGE_READ | PAGE_WRITE); + pkru_prot &= ~(PAGE_READ | PAGE_WRITE); } else if (pkru_wd && (is_user || env->cr[0] & CR0_WP_MASK)) { - prot &= ~PAGE_WRITE; + pkru_prot &= ~PAGE_WRITE; } - if ((prot & (1 << is_write1)) == 0) { + + prot &= pkru_prot; + if ((pkru_prot & (1 << is_write1)) == 0) { assert(is_write1 != 2); error_code |= PG_ERROR_PK_MASK; goto do_fault_protect; } } + if ((prot & (1 << is_write1)) == 0) { + goto do_fault_protect; + } + /* yes, it can! */ is_dirty = is_write && !(pte & PG_DIRTY_MASK); if (!(pte & PG_ACCESSED_MASK) || is_dirty) {