Re: [PATCH 1/6] target/arm: Fault on invalid TCR_ELx.TxSZ

[Peter Maydell]

On Wed, 8 Dec 2021 at 23:16, Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> Without FEAT_LVA, the behaviour of programming an invalid value
> is IMPLEMENTATION DEFINED.  With FEAT_LVA, programming an invalid
> minimum value requires a Translation fault.
>
> It is most self-consistent to choose to generate the fault always.
>
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  target/arm/helper.c | 32 ++++++++++++++++++++++----------
>  1 file changed, 22 insertions(+), 10 deletions(-)
>
> diff --git a/target/arm/helper.c b/target/arm/helper.c
> index 9b317899a6..575723d62c 100644
> --- a/target/arm/helper.c
> +++ b/target/arm/helper.c
> @@ -11129,7 +11129,7 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, 
> uint64_t va,
>  {
>      uint64_t tcr = regime_tcr(env, mmu_idx)->raw_tcr;
>      bool epd, hpd, using16k, using64k;
> -    int select, tsz, tbi, max_tsz;
> +    int select, tsz, tbi;
>
>      if (!regime_has_2_ranges(mmu_idx)) {
>          select = 0;
> @@ -11165,15 +11165,6 @@ ARMVAParameters aa64_va_parameters(CPUARMState *env, 
> uint64_t va,
>          }
>      }
>
> -    if (cpu_isar_feature(aa64_st, env_archcpu(env))) {
> -        max_tsz = 48 - using64k;
> -    } else {
> -        max_tsz = 39;
> -    }
> -
> -    tsz = MIN(tsz, max_tsz);
> -    tsz = MAX(tsz, 16);  /* TODO: ARMv8.2-LVA  */
> -

These changes are OK in themselves, but we also use the
aa64_va_parameters() calculated tsz value in the
pointer-auth code to work out the bottom bit of the
pointer auth field:

    bot_bit = 64 - param.tsz;
    top_bit = 64 - 8 * param.tbi;

Without the clamping of param.tsz to the valid range,
the guest can now program it to a value that will cause
us to have bot_bit > top_bit (eg tsz = 0). We don't
guard against that and as a result code like
extract64(test, bot_bit, top_bit - bot_bit)
will assert on the bogus length value.

(Section D5.1.5 says what the pauth code is allowed to do
if the TnSZ field is out-of-limits: it can use the value as-is,
or it can clamp it to the limit.)

-- PMM