diff mbox series

target/arm: Fixed Privileged Access Never (PAN) for aarch32

Message ID 20221027112619.2205229-1-tkutergin@gmail.com
State New
Headers show
Series target/arm: Fixed Privileged Access Never (PAN) for aarch32 | expand

Commit Message

Timofey Kutergin Oct. 27, 2022, 11:26 a.m. UTC
- Use CPSR.PAN to check for PAN state in aarch32 mode
    - throw permission fault during address translation when PAN is
      enabled and kernel tries to access user acessible page
    - ignore SCTLR_XP bit for armv7 and armv8 (conflicts with SCTLR_SPAN).

Signed-off-by: Timofey Kutergin <tkutergin@gmail.com>
---
 target/arm/helper.c | 13 +++++++++++--
 target/arm/ptw.c    | 35 ++++++++++++++++++++++++++++++-----
 2 files changed, 41 insertions(+), 7 deletions(-)

Comments

Peter Maydell Oct. 28, 2022, 6:17 p.m. UTC | #1
On Thu, 27 Oct 2022 at 12:26, Timofey Kutergin <tkutergin@gmail.com> wrote:
>
>     - Use CPSR.PAN to check for PAN state in aarch32 mode
>     - throw permission fault during address translation when PAN is
>       enabled and kernel tries to access user acessible page
>     - ignore SCTLR_XP bit for armv7 and armv8 (conflicts with SCTLR_SPAN).
>
> Signed-off-by: Timofey Kutergin <tkutergin@gmail.com>
> ---
>  target/arm/helper.c | 13 +++++++++++--
>  target/arm/ptw.c    | 35 ++++++++++++++++++++++++++++++-----
>  2 files changed, 41 insertions(+), 7 deletions(-)

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

Not sure if I'll be able to get this in before softfreeze,
but since it's a bug fix it'll be in 7.2.

thanks
-- PMM
Peter Maydell Oct. 31, 2022, 1:45 p.m. UTC | #2
On Fri, 28 Oct 2022 at 19:17, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 27 Oct 2022 at 12:26, Timofey Kutergin <tkutergin@gmail.com> wrote:
> >
> >     - Use CPSR.PAN to check for PAN state in aarch32 mode
> >     - throw permission fault during address translation when PAN is
> >       enabled and kernel tries to access user acessible page
> >     - ignore SCTLR_XP bit for armv7 and armv8 (conflicts with SCTLR_SPAN).
> >
> > Signed-off-by: Timofey Kutergin <tkutergin@gmail.com>
> > ---
> >  target/arm/helper.c | 13 +++++++++++--
> >  target/arm/ptw.c    | 35 ++++++++++++++++++++++++++++++-----
> >  2 files changed, 41 insertions(+), 7 deletions(-)
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
>
> Not sure if I'll be able to get this in before softfreeze,
> but since it's a bug fix it'll be in 7.2.



Applied to target-arm.next, thanks.

-- PMM
diff mbox series

Patch

diff --git a/target/arm/helper.c b/target/arm/helper.c
index c672903f43..4301478ed8 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10992,6 +10992,15 @@  ARMMMUIdx arm_v7m_mmu_idx_for_secstate(CPUARMState *env, bool secstate)
 }
 #endif
 
+static bool arm_pan_enabled(CPUARMState *env)
+{
+    if (is_a64(env)) {
+        return env->pstate & PSTATE_PAN;
+    } else {
+        return env->uncached_cpsr & CPSR_PAN;
+    }
+}
+
 ARMMMUIdx arm_mmu_idx_el(CPUARMState *env, int el)
 {
     ARMMMUIdx idx;
@@ -11012,7 +11021,7 @@  ARMMMUIdx arm_mmu_idx_el(CPUARMState *env, int el)
         }
         break;
     case 1:
-        if (env->pstate & PSTATE_PAN) {
+        if (arm_pan_enabled(env)) {
             idx = ARMMMUIdx_E10_1_PAN;
         } else {
             idx = ARMMMUIdx_E10_1;
@@ -11021,7 +11030,7 @@  ARMMMUIdx arm_mmu_idx_el(CPUARMState *env, int el)
     case 2:
         /* Note that TGE does not apply at EL2.  */
         if (arm_hcr_el2_eff(env) & HCR_E2H) {
-            if (env->pstate & PSTATE_PAN) {
+            if (arm_pan_enabled(env)) {
                 idx = ARMMMUIdx_E20_2_PAN;
             } else {
                 idx = ARMMMUIdx_E20_2;
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 6c5ed56a10..a82accab40 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -433,12 +433,11 @@  static bool get_level1_table_address(CPUARMState *env, ARMMMUIdx mmu_idx,
  * @mmu_idx:     MMU index indicating required translation regime
  * @ap:          The 3-bit access permissions (AP[2:0])
  * @domain_prot: The 2-bit domain access permissions
+ * @is_user: TRUE if accessing from PL0
  */
-static int ap_to_rw_prot(CPUARMState *env, ARMMMUIdx mmu_idx,
-                         int ap, int domain_prot)
+static int ap_to_rw_prot_is_user(CPUARMState *env, ARMMMUIdx mmu_idx,
+                         int ap, int domain_prot, bool is_user)
 {
-    bool is_user = regime_is_user(env, mmu_idx);
-
     if (domain_prot == 3) {
         return PAGE_READ | PAGE_WRITE;
     }
@@ -482,6 +481,20 @@  static int ap_to_rw_prot(CPUARMState *env, ARMMMUIdx mmu_idx,
     }
 }
 
+/*
+ * Translate section/page access permissions to page R/W protection flags
+ * @env:         CPUARMState
+ * @mmu_idx:     MMU index indicating required translation regime
+ * @ap:          The 3-bit access permissions (AP[2:0])
+ * @domain_prot: The 2-bit domain access permissions
+ */
+static int ap_to_rw_prot(CPUARMState *env, ARMMMUIdx mmu_idx,
+                         int ap, int domain_prot)
+{
+   return ap_to_rw_prot_is_user(env, mmu_idx, ap, domain_prot,
+                                regime_is_user(env, mmu_idx));
+}
+
 /*
  * Translate section/page access permissions to page R/W protection flags.
  * @ap:      The 2-bit simple AP (AP[2:1])
@@ -644,6 +657,7 @@  static bool get_phys_addr_v6(CPUARMState *env, S1Translate *ptw,
     hwaddr phys_addr;
     uint32_t dacr;
     bool ns;
+    int user_prot;
 
     /* Pagetable walk.  */
     /* Lookup l1 descriptor.  */
@@ -749,8 +763,10 @@  static bool get_phys_addr_v6(CPUARMState *env, S1Translate *ptw,
                 goto do_fault;
             }
             result->f.prot = simple_ap_to_rw_prot(env, mmu_idx, ap >> 1);
+            user_prot = simple_ap_to_rw_prot_is_user(ap >> 1, 1);
         } else {
             result->f.prot = ap_to_rw_prot(env, mmu_idx, ap, domain_prot);
+            user_prot = ap_to_rw_prot_is_user(env, mmu_idx, ap, domain_prot, 1);
         }
         if (result->f.prot && !xn) {
             result->f.prot |= PAGE_EXEC;
@@ -760,6 +776,14 @@  static bool get_phys_addr_v6(CPUARMState *env, S1Translate *ptw,
             fi->type = ARMFault_Permission;
             goto do_fault;
         }
+        if (regime_is_pan(env, mmu_idx) &&
+            !regime_is_user(env, mmu_idx) &&
+            user_prot &&
+            access_type != MMU_INST_FETCH) {
+            /* Privileged Access Never fault */
+            fi->type = ARMFault_Permission;
+            goto do_fault;
+        }
     }
     if (ns) {
         /* The NS bit will (as required by the architecture) have no effect if
@@ -2606,7 +2630,8 @@  static bool get_phys_addr_with_struct(CPUARMState *env, S1Translate *ptw,
     if (regime_using_lpae_format(env, mmu_idx)) {
         return get_phys_addr_lpae(env, ptw, address, access_type, false,
                                   result, fi);
-    } else if (regime_sctlr(env, mmu_idx) & SCTLR_XP) {
+    } else if (arm_feature(env, ARM_FEATURE_V7) ||
+               regime_sctlr(env, mmu_idx) & SCTLR_XP) {
         return get_phys_addr_v6(env, ptw, address, access_type, result, fi);
     } else {
         return get_phys_addr_v5(env, ptw, address, access_type, result, fi);