target/arm: PSTATE.PAN should not clear exec bits

[PATCH for-5.0] target/arm: PSTATE.PAN should not clear exec bits

Peter Maydell posted 1 patch 4 years ago

Download mbox

Test docker-mingw@fedora passed

Test docker-quick@centos7 passed

Test checkpatch passed

Test FreeBSD passed

Test asan passed

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200330170651.20901-1-peter.maydell@linaro.org

Maintainers: Peter Maydell <peter.maydell@linaro.org>

target/arm/helper.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)

Expand all Fold all

[PATCH for-5.0] target/arm: PSTATE.PAN should not clear exec bits

Posted by Peter Maydell 4 years ago

Our implementation of the PSTATE.PAN bit incorrectly cleared all
access permission bits for privileged access to memory which is
user-accessible.  It should only affect the privileged read and write
permissions; execute permission is dealt with via XN/PXN instead.

Fixes: 81636b70c226dc27d7ebc8d
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Compare the pseudocode AArch64.CheckPermission().
---
 target/arm/helper.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 163c91a1ccd..ed7eb8ab54e 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10025,9 +10025,11 @@ static int get_S1prot(CPUARMState *env, ARMMMUIdx mmu_idx, bool is_aa64,
         prot_rw = user_rw;
     } else {
         if (user_rw && regime_is_pan(env, mmu_idx)) {
-            return 0;
+            /* PAN forbids data accesses but doesn't affect insn fetch */
+            prot_rw = 0;
+        } else {
+            prot_rw = simple_ap_to_rw_prot_is_user(ap, false);
         }
-        prot_rw = simple_ap_to_rw_prot_is_user(ap, false);
     }
 
     if (ns && arm_is_secure(env) && (env->cp15.scr_el3 & SCR_SIF)) {
-- 
2.20.1

Re: [PATCH for-5.0] target/arm: PSTATE.PAN should not clear exec bits

Posted by Richard Henderson 4 years ago

On 3/30/20 10:06 AM, Peter Maydell wrote:
> Our implementation of the PSTATE.PAN bit incorrectly cleared all
> access permission bits for privileged access to memory which is
> user-accessible.  It should only affect the privileged read and write
> permissions; execute permission is dealt with via XN/PXN instead.
> 
> Fixes: 81636b70c226dc27d7ebc8d
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Compare the pseudocode AArch64.CheckPermission().
> ---
>  target/arm/helper.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)

Oops,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~