Re: [PATCH] target/riscv: Add asserts for out-of-bound access

[Alistair Francis]

On Wed, Jul 24, 2024 at 6:33 PM Atish Patra <atishp@rivosinc.com> wrote:
>
> Coverity complained about the possible out-of-bounds access with
> counter_virt/counter_virt_prev because these two arrays are
> accessed with privilege mode. However, these two arrays are accessed
> only when virt is enabled. Thus, the privilege mode can't be M mode.
>
> Add the asserts anyways to detect any wrong usage of these arrays
> in the future.
>
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Atish Patra <atishp@rivosinc.com>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
> The lore discussion can be found here
> https://lore.kernel.org/all/CAHBxVyGQHBobpf71o4Qp51iQGXKBh0Ajup=e_a95xdLF==V_WQ@mail.gmail.com/
> ---
>  target/riscv/pmu.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/target/riscv/pmu.c b/target/riscv/pmu.c
> index 3cc0b3648cad..e05ab067d2f2 100644
> --- a/target/riscv/pmu.c
> +++ b/target/riscv/pmu.c
> @@ -204,6 +204,7 @@ static void riscv_pmu_icount_update_priv(CPURISCVState 
> *env,
>      }
>
>      if (env->virt_enabled) {
> +        g_assert(env->priv <= PRV_S);
>          counter_arr = env->pmu_fixed_ctrs[1].counter_virt;
>          snapshot_prev = env->pmu_fixed_ctrs[1].counter_virt_prev;
>      } else {
> @@ -212,6 +213,7 @@ static void riscv_pmu_icount_update_priv(CPURISCVState 
> *env,
>      }
>
>      if (new_virt) {
> +        g_assert(newpriv <= PRV_S);
>          snapshot_new = env->pmu_fixed_ctrs[1].counter_virt_prev;
>      } else {
>          snapshot_new = env->pmu_fixed_ctrs[1].counter_prev;
> @@ -242,6 +244,7 @@ static void riscv_pmu_cycle_update_priv(CPURISCVState 
> *env,
>      }
>
>      if (env->virt_enabled) {
> +        g_assert(env->priv <= PRV_S);
>          counter_arr = env->pmu_fixed_ctrs[0].counter_virt;
>          snapshot_prev = env->pmu_fixed_ctrs[0].counter_virt_prev;
>      } else {
> @@ -250,6 +253,7 @@ static void riscv_pmu_cycle_update_priv(CPURISCVState 
> *env,
>      }
>
>      if (new_virt) {
> +        g_assert(newpriv <= PRV_S);
>          snapshot_new = env->pmu_fixed_ctrs[0].counter_virt_prev;
>      } else {
>          snapshot_new = env->pmu_fixed_ctrs[0].counter_prev;
>
> ---
> base-commit: daff9f7f7a457f78ce455e6abf19c2a37dfe7630
> change-id: 20240723-fixes-439b929bfbc8
> --
> Regards,
> Atish patra
>
>