target/arm: Fix MTE access checks for disabled SEL2

[PATCH] target/arm: Fix MTE access checks for disabled SEL2

Posted by Idan Horowitz 3 years, 10 months ago

While not mentioned anywhere in the actual specification text, the
HCR_EL2.ATA bit is treated as '1' when EL2 is disabled at the current
security state. This can be observed in the psuedo-code implementation
of AArch64.AllocationTagAccessIsEnabled().

Signed-off-by: Idan Horowitz <idan.horowitz@gmail.com>
---
 target/arm/helper.c    | 2 +-
 target/arm/internals.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 16c2628f8f..7d14650615 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -7176,7 +7176,7 @@ static CPAccessResult access_mte(CPUARMState *env, const ARMCPRegInfo *ri,
 {
     int el = arm_current_el(env);
 
-    if (el < 2 && arm_feature(env, ARM_FEATURE_EL2)) {
+    if (el < 2 && arm_is_el2_enabled(env)) {
         uint64_t hcr = arm_hcr_el2_eff(env);
         if (!(hcr & HCR_ATA) && (!(hcr & HCR_E2H) || !(hcr & HCR_TGE))) {
             return CP_ACCESS_TRAP_EL2;
diff --git a/target/arm/internals.h b/target/arm/internals.h
index a34be2e459..7f696cd36a 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -1094,7 +1094,7 @@ static inline bool allocation_tag_access_enabled(CPUARMState *env, int el,
         && !(env->cp15.scr_el3 & SCR_ATA)) {
         return false;
     }
-    if (el < 2 && arm_feature(env, ARM_FEATURE_EL2)) {
+    if (el < 2 && arm_is_el2_enabled(env)) {
         uint64_t hcr = arm_hcr_el2_eff(env);
         if (!(hcr & HCR_ATA) && (!(hcr & HCR_E2H) || !(hcr & HCR_TGE))) {
             return false;
-- 
2.35.1

Re: [PATCH] target/arm: Fix MTE access checks for disabled SEL2

Posted by Richard Henderson 3 years, 10 months ago

On 3/28/22 11:31, Idan Horowitz wrote:
> While not mentioned anywhere in the actual specification text, the
> HCR_EL2.ATA bit is treated as '1' when EL2 is disabled at the current
> security state. This can be observed in the psuedo-code implementation
> of AArch64.AllocationTagAccessIsEnabled().
> 
> Signed-off-by: Idan Horowitz <idan.horowitz@gmail.com>
> ---
>   target/arm/helper.c    | 2 +-
>   target/arm/internals.h | 2 +-
>   2 files changed, 2 insertions(+), 2 deletions(-)

I was immediately thinking, didn't I just fix this?
But I was patching pauth.  Anyway, good catch.

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


r~

Re: [PATCH] target/arm: Fix MTE access checks for disabled SEL2

Posted by Peter Maydell 3 years, 10 months ago

On Mon, 28 Mar 2022 at 19:04, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 3/28/22 11:31, Idan Horowitz wrote:
> > While not mentioned anywhere in the actual specification text, the
> > HCR_EL2.ATA bit is treated as '1' when EL2 is disabled at the current
> > security state. This can be observed in the psuedo-code implementation
> > of AArch64.AllocationTagAccessIsEnabled().
> >
> > Signed-off-by: Idan Horowitz <idan.horowitz@gmail.com>
> > ---
> >   target/arm/helper.c    | 2 +-
> >   target/arm/internals.h | 2 +-
> >   2 files changed, 2 insertions(+), 2 deletions(-)
>
> I was immediately thinking, didn't I just fix this?
> But I was patching pauth.  Anyway, good catch.
>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>



Applied to target-arm.next, thanks.

-- PMM