[Qemu-devel] [PATCH 13/13] target/arm: Add v8M stack checks for MSR to SP_NS

Peter Maydell posted 13 patches 7 years, 4 months ago
[Qemu-devel] [PATCH 13/13] target/arm: Add v8M stack checks for MSR to SP_NS
Posted by Peter Maydell 7 years, 4 months ago
Updating the NS stack pointer via MSR to SP_NS should include
a check whether the new SP value is below the stack limit.
No other kinds of update to the various stack pointer and
limit registers via MSR should perform a check.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/helper.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index 074f7616272..712828674fa 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10963,11 +10963,23 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val)
              * currently in handler mode or not, using the NS CONTROL.SPSEL.
              */
             bool spsel = env->v7m.control[M_REG_NS] & R_V7M_CONTROL_SPSEL_MASK;
+            bool is_psp = !arm_v7m_is_handler_mode(env) && spsel;
+            uint32_t limit;
 
             if (!env->v7m.secure) {
                 return;
             }
-            if (!arm_v7m_is_handler_mode(env) && spsel) {
+
+            limit = is_psp ? env->v7m.psplim[false] : env->v7m.msplim[false];
+
+            if (val < limit) {
+                CPUState *cs = CPU(arm_env_get_cpu(env));
+
+                cpu_restore_state(cs, GETPC(), true);
+                raise_exception(env, EXCP_STKOF, 0, 1);
+            }
+
+            if (is_psp) {
                 env->v7m.other_ss_psp = val;
             } else {
                 env->v7m.other_ss_msp = val;
-- 
2.19.0


Re: [Qemu-devel] [PATCH 13/13] target/arm: Add v8M stack checks for MSR to SP_NS
Posted by Philippe Mathieu-Daudé 7 years, 4 months ago
On 02/10/2018 18:35, Peter Maydell wrote:
> Updating the NS stack pointer via MSR to SP_NS should include
> a check whether the new SP value is below the stack limit.
> No other kinds of update to the various stack pointer and
> limit registers via MSR should perform a check.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>

> ---
>  target/arm/helper.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 074f7616272..712828674fa 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -10963,11 +10963,23 @@ void HELPER(v7m_msr)(CPUARMState *env, uint32_t maskreg, uint32_t val)
>               * currently in handler mode or not, using the NS CONTROL.SPSEL.
>               */
>              bool spsel = env->v7m.control[M_REG_NS] & R_V7M_CONTROL_SPSEL_MASK;
> +            bool is_psp = !arm_v7m_is_handler_mode(env) && spsel;
> +            uint32_t limit;
>  
>              if (!env->v7m.secure) {
>                  return;
>              }
> -            if (!arm_v7m_is_handler_mode(env) && spsel) {
> +
> +            limit = is_psp ? env->v7m.psplim[false] : env->v7m.msplim[false];
> +
> +            if (val < limit) {
> +                CPUState *cs = CPU(arm_env_get_cpu(env));
> +
> +                cpu_restore_state(cs, GETPC(), true);
> +                raise_exception(env, EXCP_STKOF, 0, 1);
> +            }
> +
> +            if (is_psp) {
>                  env->v7m.other_ss_psp = val;
>              } else {
>                  env->v7m.other_ss_msp = val;
> 

Re: [Qemu-devel] [PATCH 13/13] target/arm: Add v8M stack checks for MSR to SP_NS
Posted by Richard Henderson 7 years, 4 months ago
On 10/2/18 11:35 AM, Peter Maydell wrote:
> Updating the NS stack pointer via MSR to SP_NS should include
> a check whether the new SP value is below the stack limit.
> No other kinds of update to the various stack pointer and
> limit registers via MSR should perform a check.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  target/arm/helper.c | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)

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

r~