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

[Atish Kumar Patra]

On Thu, Jul 25, 2024 at 10:12 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> 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>
>
> Fixes: Coverity CID 1558459
> Fixes: Coverity CID 1558462
>
> > ---
> > 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);
>
> Don't we need this assert for !env->virt_enabled as well?
>

For that case, it uses counter and counter_prev which is array size of 4.
The assert was in the other case just to avoid wrong invocation in the
future with PRV_M while the array size is 2.

> Alistair