[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PATCH] target/arm: Fix bug in memory translation for executable Realm m
From: |
Matti Schulze |
Subject: |
[PATCH] target/arm: Fix bug in memory translation for executable Realm memory pages |
Date: |
Tue, 22 Aug 2023 18:17:55 +0200 |
This patch fixes a bug in the memory translation for target/arm.
If in realm space, e.g., R-EL2 executing code from an executable
memory page currently results in a level 3 permission fault.
As we cannot access secure memory from an insecure space,
QEMU checks on each memory translation if the in_space is secure va
!ptw->in_secure.
If this is the case we always set the NS bit in the memory attributes
to prevent ever reading secure memory from an insecure space.
This collides with FEAT_RME, since if the system is in realm space,
!ptw->in_secure also applies, and thus QEMU sets the NS bit,
meaning that the memory will be translated into insecure space.
Fetching the instruction from this memory space leads to a fault,
as you cannot execute NS instructions from a realm context.
To fix this we introduce the ptw->in_realm variable mirroring the
behavior for in_secure and only set the NS bit if both do not apply.
Signed-off-by: Matti Schulze <matti.schulze@fau.de>
---
target/arm/cpu.h | 6 ++++++
target/arm/ptw.c | 17 +++++++++++++++--
2 files changed, 21 insertions(+), 2 deletions(-)
diff --git a/target/arm/cpu.h b/target/arm/cpu.h
index 88e5accda6..ff7f8f511d 100644
--- a/target/arm/cpu.h
+++ b/target/arm/cpu.h
@@ -2436,6 +2436,12 @@ static inline bool arm_space_is_secure(ARMSecuritySpace
space)
return space == ARMSS_Secure || space == ARMSS_Root;
}
+/* Return true if @space is Realm space */
+static inline bool arm_space_is_realm(ARMSecuritySpace space)
+{
+ return space == ARMSS_Realm;
+}
+
/* Return the ARMSecuritySpace for @secure, assuming !RME or EL[0-2]. */
static inline ARMSecuritySpace arm_secure_to_space(bool secure)
{
diff --git a/target/arm/ptw.c b/target/arm/ptw.c
index 8f94100c61..db1b5a7fbd 100644
--- a/target/arm/ptw.c
+++ b/target/arm/ptw.c
@@ -58,6 +58,13 @@ typedef struct S1Translate {
* this field is updated accordingly.
*/
bool in_secure;
+ /*
+ * in_realm: whether the translation regime is Realm
+ * This is always requal to arm_space_in_realm(in_space).
+ * If a Realm ptw is "downgraded" to a NonSecure by an NSTable bit
+ * this field is updated accordingly.
+ */
+ bool in_realm;
/*
* in_debug: is this a QEMU debug access (gdbstub, etc)? Debug
* accesses will not update the guest page table access flags
@@ -535,6 +542,7 @@ static bool S1_ptw_translate(CPUARMState *env, S1Translate
*ptw,
.in_mmu_idx = s2_mmu_idx,
.in_ptw_idx = ptw_idx_for_stage_2(env, s2_mmu_idx),
.in_secure = arm_space_is_secure(s2_space),
+ .in_realm = arm_space_is_realm(s2_space),
.in_space = s2_space,
.in_debug = true,
};
@@ -724,7 +732,7 @@ static uint64_t arm_casq_ptw(CPUARMState *env, uint64_t
old_val,
fi->s2addr = ptw->out_virt;
fi->stage2 = true;
fi->s1ptw = true;
- fi->s1ns = !ptw->in_secure;
+ fi->s1ns = !ptw->in_secure && !ptw->in_realm;
return 0;
}
@@ -1767,6 +1775,7 @@ static bool get_phys_addr_lpae(CPUARMState *env,
S1Translate *ptw,
QEMU_BUILD_BUG_ON(ARMMMUIdx_Stage2_S + 1 != ARMMMUIdx_Stage2);
ptw->in_ptw_idx += 1;
ptw->in_secure = false;
+ ptw->in_realm = false;
ptw->in_space = ARMSS_NonSecure;
}
@@ -1872,7 +1881,7 @@ static bool get_phys_addr_lpae(CPUARMState *env,
S1Translate *ptw,
*/
attrs = new_descriptor & (MAKE_64BIT_MASK(2, 10) | MAKE_64BIT_MASK(50,
14));
if (!regime_is_stage2(mmu_idx)) {
- attrs |= !ptw->in_secure << 5; /* NS */
+ attrs |= (!ptw->in_secure && !ptw->in_realm) << 5; /* NS */
if (!param.hpd) {
attrs |= extract64(tableattrs, 0, 2) << 53; /* XN, PXN */
/*
@@ -3139,6 +3148,7 @@ static bool get_phys_addr_twostage(CPUARMState *env,
S1Translate *ptw,
ptw->in_mmu_idx = ipa_secure ? ARMMMUIdx_Stage2_S : ARMMMUIdx_Stage2;
ptw->in_secure = ipa_secure;
ptw->in_space = ipa_space;
+ ptw->in_realm = arm_space_is_realm(ipa_space);
ptw->in_ptw_idx = ptw_idx_for_stage_2(env, ptw->in_mmu_idx);
/*
@@ -3371,6 +3381,7 @@ bool get_phys_addr_with_secure(CPUARMState *env,
target_ulong address,
S1Translate ptw = {
.in_mmu_idx = mmu_idx,
.in_secure = is_secure,
+ .in_realm = false,
.in_space = arm_secure_to_space(is_secure),
};
return get_phys_addr_gpc(env, &ptw, address, access_type, result, fi);
@@ -3443,6 +3454,7 @@ bool get_phys_addr(CPUARMState *env, target_ulong address,
ptw.in_space = ss;
ptw.in_secure = arm_space_is_secure(ss);
+ ptw.in_realm = arm_space_is_realm(ss);
return get_phys_addr_gpc(env, &ptw, address, access_type, result, fi);
}
@@ -3457,6 +3469,7 @@ hwaddr arm_cpu_get_phys_page_attrs_debug(CPUState *cs,
vaddr addr,
.in_mmu_idx = mmu_idx,
.in_space = ss,
.in_secure = arm_space_is_secure(ss),
+ .in_realm = arm_space_is_realm(ss),
.in_debug = true,
};
GetPhysAddrResult res = {};
--
2.41.0
- [PATCH] target/arm: Fix bug in memory translation for executable Realm memory pages,
Matti Schulze <=