Re: [PATCH] target/microblaze: Fix possible array out of bounds in mmu_write()

[AlexChen]

On 2020/11/6 22:16, Philippe Mathieu-Daudé wrote:
> On 11/3/20 8:46 AM, AlexChen wrote:
>> The size of env->mmu.regs is 3, but the range of 'rn' is [0, 5].
>> To avoid data access out of bounds, only if 'rn' is less than 3, we
>> can print env->mmu.regs[rn]. In other cases, we can print
>> env->mmu.regs[MMU_R_TLBX].
>>
>> Reported-by: Euler Robot <euler.robot@huawei.com>
>> Signed-off-by: Alex Chen <alex.chen@huawei.com>
>> ---
>>  target/microblaze/mmu.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/microblaze/mmu.c b/target/microblaze/mmu.c
>> index 1dbbb271c4..917ad6d69e 100644
>> --- a/target/microblaze/mmu.c
>> +++ b/target/microblaze/mmu.c
>> @@ -234,7 +234,8 @@ void mmu_write(CPUMBState *env, bool ext, uint32_t rn, 
>> uint32_t v)
>>      unsigned int i;
>>
>>      qemu_log_mask(CPU_LOG_MMU,
>> -                  "%s rn=%d=%x old=%x\n", __func__, rn, v, 
>> env->mmu.regs[rn]);
>> +                  "%s rn=%d=%x old=%x\n", __func__, rn, v,
>> +                  rn < 3 ? env->mmu.regs[rn] : env->mmu.regs[MMU_R_TLBX]);
> 
> Nack. If rn >= ARRAY_SIZE(env->mmu.regs), then don't displays it.
> Else it is confuse to see a value unrelated to the MMU index used...
> 
Hi Philippe,

Thanks for your review.
The env->mmu.regs[MMU_R_TLBX] is used when rn >= ARRAY_SIZE(env->mmu.regs),
can we change the description of the log as follows so that it doesn't confuse 
us?

---
 target/microblaze/mmu.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/microblaze/mmu.c b/target/microblaze/mmu.c
index 1dbbb271c4..14863ed8d1 100644
--- a/target/microblaze/mmu.c
+++ b/target/microblaze/mmu.c
@@ -234,7 +234,9 @@ void mmu_write(CPUMBState *env, bool ext, uint32_t rn, 
uint32_t v)
     unsigned int i;

     qemu_log_mask(CPU_LOG_MMU,
-                  "%s rn=%d=%x old=%x\n", __func__, rn, v, env->mmu.regs[rn]);
+                  "%s rn=%d=%x %s=%x\n", __func__, rn, v,
+                  rn < 3 ? "old" : "regs[MMU_R_TLBX]",
+                  rn < 3 ? env->mmu.regs[rn] : env->mmu.regs[MMU_R_TLBX]);

     if (cpu->cfg.mmu < 2 || !cpu->cfg.mmu_tlb_access) {
         qemu_log_mask(LOG_GUEST_ERROR, "MMU access on MMU-less system\n");

Thanks,
Alex