Re: [Qemu-riscv] [Qemu-devel] [PATCHv3 4/5] RISC-V: Fix a PMP bug where it succeeds even if PMP entry is off

[Alistair Francis]

On Tue, May 21, 2019 at 3:47 AM Hesham Almatary
<address@hidden> wrote:
>
> The current implementation returns 1 (PMP check success) if the address is in
> range even if the PMP entry is off. This is a bug.
>
> For example, if there is a PMP check in S-Mode which is in range, but its PMP
> entry is off, this will succeed, which it should not.
>
> The patch fixes this bug by only checking the PMP permissions if the address 
> is
> in range and its corresponding PMP entry it not off. Otherwise, it will keep
> the ret = -1 which will be checked and handled correctly at the end of the
> function.
>
> Signed-off-by: Hesham Almatary <address@hidden>

Reviewed-by: Alistair Francis <address@hidden>

Alistair

> ---
>  target/riscv/pmp.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/target/riscv/pmp.c b/target/riscv/pmp.c
> index b11c4ae22f..8668f0dd7c 100644
> --- a/target/riscv/pmp.c
> +++ b/target/riscv/pmp.c
> @@ -259,11 +259,12 @@ bool pmp_hart_has_privs(CPURISCVState *env, 
> target_ulong addr,
>          /* fully inside */
>          const uint8_t a_field =
>              pmp_get_a_field(env->pmp_state.pmp[i].cfg_reg);
> -        if ((s + e) == 2) {
> -            if (PMP_AMATCH_OFF == a_field) {
> -                return 1;
> -            }
>
> +        /*
> +         * If the PMP entry is not off and the address is in range, do the 
> priv
> +         * check
> +         */
> +        if (((s + e) == 2) && (PMP_AMATCH_OFF != a_field)) {
>              allowed_privs = PMP_READ | PMP_WRITE | PMP_EXEC;
>              if ((env->priv != PRV_M) || pmp_is_locked(env, i)) {
>                  allowed_privs &= env->pmp_state.pmp[i].cfg_reg;
> --
> 2.17.1
>
>