[PATCH for-5.1 0/3] target/arm: mte+pauth fixes

[PATCH for-5.1 0/3] target/arm: mte+pauth fixes

[Richard Henderson]

A couple of last minute fixes for MTE:

 (1) Peter pointed out that EL3's SCR.ATA needs to be set when
     we're booting a kernel directly.  Similarly for API & APK.

 (2) Vincenzo pointed out that with RRND=1, we can't rely on
     RGSR having being initialized.

     I suppose the only follow-on question here is whether it is
     better to minimize the number of calls to qemu_guest_getrandom,
     or instead to name that our IMPDEF algorithm and use it for
     every call to IRG.  We already have other user-space available
     RNG instructions that can drain the entropy pool, so this is
     not really different.


r~


Richard Henderson (3):
  hw/arm/boot: Fix PAUTH for EL3 direct kernel boot
  hw/arm/boot: Fix MTE for EL3 direct kernel boot
  target/arm: Improve IMPDEF algorithm for IRG

 hw/arm/boot.c           |  6 ++++++
 target/arm/mte_helper.c | 37 ++++++++++++++++++++++++++++++-------
 2 files changed, 36 insertions(+), 7 deletions(-)

-- 
2.25.1

[PATCH 1/3] hw/arm/boot: Fix PAUTH for EL3 direct kernel boot

[Richard Henderson]

When booting an EL3 cpu with -kernel, we set up EL3 and then
drop down to EL2.  We need to enable access to v8.3-PAuth
keys and instructions at EL3 before doing so.

Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 hw/arm/boot.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index fef4072db1..c44fd3382d 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -736,6 +736,9 @@ static void do_cpu_reset(void *opaque)
                     } else {
                         env->pstate = PSTATE_MODE_EL1h;
                     }
+                    if (cpu_isar_feature(aa64_pauth, cpu)) {
+                        env->cp15.scr_el3 |= SCR_API | SCR_APK;
+                    }
                     /* AArch64 kernels never boot in secure mode */
                     assert(!info->secure_boot);
                     /* This hook is only supported for AArch32 currently:
-- 
2.25.1

[PATCH 2/3] hw/arm/boot: Fix MTE for EL3 direct kernel boot

[Richard Henderson]

When booting an EL3 cpu with -kernel, we set up EL3 and then
drop down to EL2.  We need to enable access to v8.5-MemTag
tag allocation at EL3 before doing so.

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 hw/arm/boot.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/arm/boot.c b/hw/arm/boot.c
index c44fd3382d..3e9816af80 100644
--- a/hw/arm/boot.c
+++ b/hw/arm/boot.c
@@ -739,6 +739,9 @@ static void do_cpu_reset(void *opaque)
                     if (cpu_isar_feature(aa64_pauth, cpu)) {
                         env->cp15.scr_el3 |= SCR_API | SCR_APK;
                     }
+                    if (cpu_isar_feature(aa64_mte, cpu)) {
+                        env->cp15.scr_el3 |= SCR_ATA;
+                    }
                     /* AArch64 kernels never boot in secure mode */
                     assert(!info->secure_boot);
                     /* This hook is only supported for AArch32 currently:
-- 
2.25.1

[PATCH 3/3] target/arm: Improve IMPDEF algorithm for IRG

[Richard Henderson]

When GCR_EL1.RRND==1, the choosing of the random value is IMPDEF,
and the kernel is not expected to have set RGSR_EL1.  Force a
non-zero value into SEED, so that we do not continually return
the same tag.

Reported-by: Vincenzo Frascino <vincenzo.frascino@arm.com>
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
 target/arm/mte_helper.c | 37 ++++++++++++++++++++++++++++++-------
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/target/arm/mte_helper.c b/target/arm/mte_helper.c
index 5ea57d487a..104752041f 100644
--- a/target/arm/mte_helper.c
+++ b/target/arm/mte_helper.c
@@ -24,6 +24,8 @@
 #include "exec/ram_addr.h"
 #include "exec/cpu_ldst.h"
 #include "exec/helper-proto.h"
+#include "qapi/error.h"
+#include "qemu/guest-random.h"
 
 
 static int choose_nonexcluded_tag(int tag, int offset, uint16_t exclude)
@@ -211,16 +213,37 @@ static uint8_t *allocation_tag_mem(CPUARMState *env, int ptr_mmu_idx,
 
 uint64_t HELPER(irg)(CPUARMState *env, uint64_t rn, uint64_t rm)
 {
-    int rtag;
-
-    /*
-     * Our IMPDEF choice for GCR_EL1.RRND==1 is to behave as if
-     * GCR_EL1.RRND==0, always producing deterministic results.
-     */
     uint16_t exclude = extract32(rm | env->cp15.gcr_el1, 0, 16);
+    int rrnd = extract32(env->cp15.gcr_el1, 16, 1);
     int start = extract32(env->cp15.rgsr_el1, 0, 4);
     int seed = extract32(env->cp15.rgsr_el1, 8, 16);
-    int offset, i;
+    int offset, i, rtag;
+
+    /*
+     * Our IMPDEF choice for GCR_EL1.RRND==1 is to continue to use the
+     * deterministic algorithm.  Except that with RRND==1 the kernel is
+     * not required to have set RGSR_EL1.SEED != 0, which is required for
+     * the deterministic algorithm to function.  So we force a non-zero
+     * SEED for that case.
+     */
+    if (unlikely(seed == 0) && rrnd) {
+        do {
+            Error *err = NULL;
+            uint16_t two;
+
+            if (qemu_guest_getrandom(&two, sizeof(two), &err) < 0) {
+                /*
+                 * Failed, for unknown reasons in the crypto subsystem.
+                 * Best we can do is log the reason and use a constant seed.
+                 */
+                qemu_log_mask(LOG_UNIMP, "IRG: Crypto failure: %s\n",
+                              error_get_pretty(err));
+                error_free(err);
+                two = 1;
+            }
+            seed = two;
+        } while (seed == 0);
+    }
 
     /* RandomTag */
     for (i = offset = 0; i < 4; ++i) {
-- 
2.25.1

Re: [PATCH for-5.1 0/3] target/arm: mte+pauth fixes

[Peter Maydell]

On Fri, 24 Jul 2020 at 17:38, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> A couple of last minute fixes for MTE:
>
>  (1) Peter pointed out that EL3's SCR.ATA needs to be set when
>      we're booting a kernel directly.  Similarly for API & APK.
>
>  (2) Vincenzo pointed out that with RRND=1, we can't rely on
>      RGSR having being initialized.
>
>      I suppose the only follow-on question here is whether it is
>      better to minimize the number of calls to qemu_guest_getrandom,
>      or instead to name that our IMPDEF algorithm and use it for
>      every call to IRG.  We already have other user-space available
>      RNG instructions that can drain the entropy pool, so this is
>      not really different.
>



Applied to target-arm.next, thanks.

-- PMM