The current behavior of v8m_security_lookup in helper.c only checks whether the
IDAU specifies a higher security if the SAU is enabled. If SAU.ALLNS is set to
1, this will lead to addresses being treated as non-secure, even though the
IDAU indicates that they must be secure.

This patch changes the behavior to also check the IDAU if the SAU is currently
disabled.

Signed-off-by: Thomas Roth <code@stacksmashing.net>
---
 target/arm/helper.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/target/arm/helper.c b/target/arm/helper.c
index f00c141ef9..34fe7f21bf 100644
--- a/target/arm/helper.c
+++ b/target/arm/helper.c
@@ -10479,17 +10479,17 @@ static void v8m_security_lookup(CPUARMState
*env, uint32_t address,
                 }
             }
         }
+        break;
+    }

-        /* The IDAU will override the SAU lookup results if it specifies
-         * higher security than the SAU does.
-         */
-        if (!idau_ns) {
-            if (sattrs->ns || (!idau_nsc && sattrs->nsc)) {
-                sattrs->ns = false;
-                sattrs->nsc = idau_nsc;
-            }
+    /* The IDAU will override the SAU lookup results if it specifies
+     * higher security than the SAU does.
+     */
+    if (!idau_ns) {
+        if (sattrs->ns || (!idau_nsc && sattrs->nsc)) {
+            sattrs->ns = false;
+            sattrs->nsc = idau_nsc;
         }
-        break;
     }
 }

-- 
2.17.2

On Sun, 20 Jan 2019 at 14:51, Thomas Roth <code@stacksmashing.net> wrote:
>
> The current behavior of v8m_security_lookup in helper.c only checks whether the
> IDAU specifies a higher security if the SAU is enabled. If SAU.ALLNS is set to
> 1, this will lead to addresses being treated as non-secure, even though the
> IDAU indicates that they must be secure.
>
> This patch changes the behavior to also check the IDAU if the SAU is currently
> disabled.
>
> Signed-off-by: Thomas Roth <code@stacksmashing.net>

Oops, yes. In the v8M Arm ARM the pseudocode function for this
is SecurityCheck(), and we should indeed not ignore the IDAU for
the SAU disabled path.

Applied to target-arm.next, thanks. (I'll tweak the commit message
to add the Arm ARM reference and make the block comment follow
our current coding style, since we're moving it anyway.)

thanks
-- PMM