target/arm: Don't enforce NSE,NS check for EL3->EL3 returns

[PATCH] target/arm: Don't enforce NSE,NS check for EL3->EL3 returns

Posted by Peter Maydell 5 months, 2 weeks ago

In the Arm ARM, rule R_TYTWB that defines illegal exception return cases
includes the case:
 If FEAT_RME is implemented, then if SCR_EL3.{NSE, NS} is {1, 0}, an
 exception return from EL3 to a lower Exception level

Our implementation of this check fails to check that the
return is to a lower exception level, so it will incorrectly fire on
EL3->EL3 exception returns.

Fix the check condition. This requires us to move it further
down in the function to a point where we know the new_el value.

Fixes: 35aa6715ddcd9 ("target/arm: Catch illegal-exception-return from EL3 with bad NSE/NS")
Cc: qemu-stable@nongnu.org
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/3016
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/tcg/helper-a64.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/target/arm/tcg/helper-a64.c b/target/arm/tcg/helper-a64.c
index 4f618ae390e..cb02cee51b2 100644
--- a/target/arm/tcg/helper-a64.c
+++ b/target/arm/tcg/helper-a64.c
@@ -656,15 +656,6 @@ void HELPER(exception_return)(CPUARMState *env, uint64_t new_pc)
         spsr &= ~PSTATE_SS;
     }
 
-    /*
-     * FEAT_RME forbids return from EL3 with an invalid security state.
-     * We don't need an explicit check for FEAT_RME here because we enforce
-     * in scr_write() that you can't set the NSE bit without it.
-     */
-    if (cur_el == 3 && (env->cp15.scr_el3 & (SCR_NS | SCR_NSE)) == SCR_NSE) {
-        goto illegal_return;
-    }
-
     new_el = el_from_spsr(spsr);
     if (new_el == -1) {
         goto illegal_return;
@@ -676,6 +667,17 @@ void HELPER(exception_return)(CPUARMState *env, uint64_t new_pc)
         goto illegal_return;
     }
 
+    /*
+     * FEAT_RME forbids return from EL3 to a lower exception level
+     * with an invalid security state.
+     * We don't need an explicit check for FEAT_RME here because we enforce
+     * in scr_write() that you can't set the NSE bit without it.
+     */
+    if (cur_el == 3 && new_el < 3 &&
+        (env->cp15.scr_el3 & (SCR_NS | SCR_NSE)) == SCR_NSE) {
+        goto illegal_return;
+    }
+
     if (new_el != 0 && arm_el_is_aa64(env, new_el) != return_to_aa64) {
         /* Return to an EL which is configured for a different register width */
         goto illegal_return;
-- 
2.43.0

Re: [PATCH] target/arm: Don't enforce NSE,NS check for EL3->EL3 returns

Posted by Richard Henderson 5 months, 2 weeks ago

On 7/4/25 10:56, Peter Maydell wrote:
> In the Arm ARM, rule R_TYTWB that defines illegal exception return cases
> includes the case:
>   If FEAT_RME is implemented, then if SCR_EL3.{NSE, NS} is {1, 0}, an
>   exception return from EL3 to a lower Exception level
> 
> Our implementation of this check fails to check that the
> return is to a lower exception level, so it will incorrectly fire on
> EL3->EL3 exception returns.
> 
> Fix the check condition. This requires us to move it further
> down in the function to a point where we know the new_el value.
> 
> Fixes: 35aa6715ddcd9 ("target/arm: Catch illegal-exception-return from EL3 with bad NSE/NS")
> Cc:qemu-stable@nongnu.org
> Resolves:https://gitlab.com/qemu-project/qemu/-/issues/3016
> Signed-off-by: Peter Maydell<peter.maydell@linaro.org>
> ---
>   target/arm/tcg/helper-a64.c | 20 +++++++++++---------
>   1 file changed, 11 insertions(+), 9 deletions(-)

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

r~