[PATCH] target/arm: Add assert to arm_to_core_mmu_idx()

[PATCH] target/arm: Add assert to arm_to_core_mmu_idx()

[Peter Maydell]

Before commit f76cee647c ("target/arm: Introduce mmu indexes for
GCS") it was impossible for arm_to_core_mmu_idx() to return an
invalid core MMU index, because NB_MMU_MODES was 16 and
ARM_MMU_IDX_COREIDX_MASK was 0xf.

That commit raises ARM_MMU_IDX_COREIDX_MASK to 0x1f and NB_MMU_MODES
to 22, so it's now possible for a bogus Arm mmu index to result in an
out of range core mmu index (which can then get used as an array
index in the CPUTLB struct arrays). Coverity complains that this
might result in an out-of-bounds access.

The out-of-bounds access can't happen because we construct all the
ARMMMUIdx values we will use for TLBs to have valid core MMU indexes
in the COREIDX field.  But we can add an assert() so that if we ever
do end up operating on a corrupted or wrong ARMMMUIdx value we get an
assert rather than silently indexing off the end of an array. This
should also make Coverity happier.

Coverity: CID 1641404
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 target/arm/internals.h | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/arm/internals.h b/target/arm/internals.h
index f539bbe58e1..026548ec34f 100644
--- a/target/arm/internals.h
+++ b/target/arm/internals.h
@@ -968,7 +968,9 @@ bool arm_cpu_tlb_fill_align(CPUState *cs, CPUTLBEntryFull *out, vaddr addr,
 
 static inline int arm_to_core_mmu_idx(ARMMMUIdx mmu_idx)
 {
-    return mmu_idx & ARM_MMU_IDX_COREIDX_MASK;
+    int coreidx = mmu_idx & ARM_MMU_IDX_COREIDX_MASK;
+    assert(coreidx < NB_MMU_MODES);
+    return coreidx;
 }
 
 static inline ARMMMUIdx core_to_arm_mmu_idx(CPUARMState *env, int mmu_idx)
-- 
2.43.0

Re: [PATCH] target/arm: Add assert to arm_to_core_mmu_idx()

[Philippe Mathieu-Daudé]

On 23/10/25 12:13, Peter Maydell wrote:
> Before commit f76cee647c ("target/arm: Introduce mmu indexes for
> GCS") it was impossible for arm_to_core_mmu_idx() to return an
> invalid core MMU index, because NB_MMU_MODES was 16 and
> ARM_MMU_IDX_COREIDX_MASK was 0xf.
> 
> That commit raises ARM_MMU_IDX_COREIDX_MASK to 0x1f and NB_MMU_MODES
> to 22, so it's now possible for a bogus Arm mmu index to result in an
> out of range core mmu index (which can then get used as an array
> index in the CPUTLB struct arrays). Coverity complains that this
> might result in an out-of-bounds access.
> 
> The out-of-bounds access can't happen because we construct all the
> ARMMMUIdx values we will use for TLBs to have valid core MMU indexes
> in the COREIDX field.  But we can add an assert() so that if we ever
> do end up operating on a corrupted or wrong ARMMMUIdx value we get an
> assert rather than silently indexing off the end of an array. This
> should also make Coverity happier.
> 
> Coverity: CID 1641404
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   target/arm/internals.h | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH] target/arm: Add assert to arm_to_core_mmu_idx()

[Richard Henderson]

On 10/23/25 05:13, Peter Maydell wrote:
> Before commit f76cee647c ("target/arm: Introduce mmu indexes for
> GCS") it was impossible for arm_to_core_mmu_idx() to return an
> invalid core MMU index, because NB_MMU_MODES was 16 and
> ARM_MMU_IDX_COREIDX_MASK was 0xf.
> 
> That commit raises ARM_MMU_IDX_COREIDX_MASK to 0x1f and NB_MMU_MODES
> to 22, so it's now possible for a bogus Arm mmu index to result in an
> out of range core mmu index (which can then get used as an array
> index in the CPUTLB struct arrays). Coverity complains that this
> might result in an out-of-bounds access.
> 
> The out-of-bounds access can't happen because we construct all the
> ARMMMUIdx values we will use for TLBs to have valid core MMU indexes
> in the COREIDX field.  But we can add an assert() so that if we ever
> do end up operating on a corrupted or wrong ARMMMUIdx value we get an
> assert rather than silently indexing off the end of an array. This
> should also make Coverity happier.
> 
> Coverity: CID 1641404
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>   target/arm/internals.h | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/target/arm/internals.h b/target/arm/internals.h
> index f539bbe58e1..026548ec34f 100644
> --- a/target/arm/internals.h
> +++ b/target/arm/internals.h
> @@ -968,7 +968,9 @@ bool arm_cpu_tlb_fill_align(CPUState *cs, CPUTLBEntryFull *out, vaddr addr,
>   
>   static inline int arm_to_core_mmu_idx(ARMMMUIdx mmu_idx)
>   {
> -    return mmu_idx & ARM_MMU_IDX_COREIDX_MASK;
> +    int coreidx = mmu_idx & ARM_MMU_IDX_COREIDX_MASK;
> +    assert(coreidx < NB_MMU_MODES);
> +    return coreidx;
>   }
>   
>   static inline ARMMMUIdx core_to_arm_mmu_idx(CPUARMState *env, int mmu_idx)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~