target/arm: Fix AddPAC error indication

The definition of top_bit used in this function is one higher
than that used in the Arm ARM psuedo-code, which put the error
indication at top_bit - 1 at the wrong place, which meant that
it wasn't visible to Auth.

Fixing the definition of top_bit requires more changes, because
its most common use is for the count of bits in top_bit:bot_bit,
which would then need to be computed as top_bit - bot_bit + 1.

For now, prefer the minimal fix to the error indication alone.

Fixes: 63ff0ca94cb
Reported-by: Derrick McKee <derrick.mckee@gmail.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
Message-id: 20200728195706.11087-1-richard.henderson@linaro.org
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
[PMM: added comment about the divergence from the pseudocode]
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

3 files changed, 39 insertions, 2 deletions

diff --git a/target/arm/pauth_helper.c b/target/arm/pauth_helper.c
index b909630317..6dbab03768 100644
--- a/target/arm/pauth_helper.c
+++ b/target/arm/pauth_helper.c
@@ -300,7 +300,11 @@ static uint64_t pauth_addpac(CPUARMState *env, uint64_t ptr, uint64_t modifier,
      */
     test = sextract64(ptr, bot_bit, top_bit - bot_bit);
     if (test != 0 && test != -1) {
-        pac ^= MAKE_64BIT_MASK(top_bit - 1, 1);
+        /*
+         * Note that our top_bit is one greater than the pseudocode's
+         * version, hence "- 2" here.
+         */
+        pac ^= MAKE_64BIT_MASK(top_bit - 2, 1);
     }
 
     /*
diff --git a/tests/tcg/aarch64/Makefile.target b/tests/tcg/aarch64/Makefile.target
index b617f2ac7e..e7249915e7 100644
--- a/tests/tcg/aarch64/Makefile.target
+++ b/tests/tcg/aarch64/Makefile.target
@@ -19,7 +19,7 @@ run-fcvt: fcvt
 
 # Pauth Tests
 ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_ARMV8_3),)
-AARCH64_TESTS += pauth-1 pauth-2 pauth-4
+AARCH64_TESTS += pauth-1 pauth-2 pauth-4 pauth-5
 pauth-%: CFLAGS += -march=armv8.3-a
 run-pauth-%: QEMU_OPTS += -cpu max
 run-plugin-pauth-%: QEMU_OPTS += -cpu max
diff --git a/tests/tcg/aarch64/pauth-5.c b/tests/tcg/aarch64/pauth-5.c
new file mode 100644
index 0000000000..67c257918b
--- /dev/null
+++ b/tests/tcg/aarch64/pauth-5.c
@@ -0,0 +1,33 @@
+#include <assert.h>
+
+static int x;
+
+int main()
+{
+    int *p0 = &x, *p1, *p2, *p3;
+    unsigned long salt = 0;
+
+    /*
+     * With TBI enabled and a 48-bit VA, there are 7 bits of auth, and so
+     * a 1/128 chance of auth = pac(ptr,key,salt) producing zero.
+     * Find a salt that creates auth != 0.
+     */
+    do {
+        salt++;
+        asm("pacda %0, %1" : "=r"(p1) : "r"(salt), "0"(p0));
+    } while (p0 == p1);
+
+    /*
+     * This pac must fail, because the input pointer bears an encryption,
+     * and so is not properly extended within bits [55:47].  This will
+     * toggle bit 54 in the output...
+     */
+    asm("pacda %0, %1" : "=r"(p2) : "r"(salt), "0"(p1));
+
+    /* ... so that the aut must fail, setting bit 53 in the output ... */
+    asm("autda %0, %1" : "=r"(p3) : "r"(salt), "0"(p2));
+
+    /* ... which means this equality must not hold. */
+    assert(p3 != p0);
+    return 0;
+}