* [PATCH] target/microblaze: Fix possible array out of bounds in mmu_write()
@ 2020-11-03 7:46 ` AlexChen
0 siblings, 0 replies; 8+ messages in thread
From: AlexChen @ 2020-11-03 7:46 UTC (permalink / raw)
To: edgar.iglesias; +Cc: QEMU Trivial, qemu-devel, zhang.zhanghailiang
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]);
if (cpu->cfg.mmu < 2 || !cpu->cfg.mmu_tlb_access) {
qemu_log_mask(LOG_GUEST_ERROR, "MMU access on MMU-less system\n");
--
2.19.1
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] target/microblaze: Fix possible array out of bounds in mmu_write()
2020-11-03 7:46 ` AlexChen
(?)
@ 2020-11-04 10:15 ` Thomas Huth
-1 siblings, 0 replies; 8+ messages in thread
From: Thomas Huth @ 2020-11-04 10:15 UTC (permalink / raw)
To: AlexChen, edgar.iglesias; +Cc: QEMU Trivial, qemu-devel, zhang.zhanghailiang
On 03/11/2020 08.46, 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].
... since env->mmu.regs[MMU_R_TLBX] is used in the other cases in this
function. Makes sense, indeed.
> 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]);
>
> if (cpu->cfg.mmu < 2 || !cpu->cfg.mmu_tlb_access) {
> qemu_log_mask(LOG_GUEST_ERROR, "MMU access on MMU-less system\n");
>
Reviewed-by: Thomas Huth <thuth@redhat.com>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] target/microblaze: Fix possible array out of bounds in mmu_write()
2020-11-03 7:46 ` AlexChen
@ 2020-11-06 10:23 ` Edgar E. Iglesias
-1 siblings, 0 replies; 8+ messages in thread
From: Edgar E. Iglesias @ 2020-11-06 10:23 UTC (permalink / raw)
To: AlexChen; +Cc: qemu-devel, QEMU Trivial, zhang.zhanghailiang
On Tue, Nov 03, 2020 at 03:46:02PM +0800, 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>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.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]);
>
> if (cpu->cfg.mmu < 2 || !cpu->cfg.mmu_tlb_access) {
> qemu_log_mask(LOG_GUEST_ERROR, "MMU access on MMU-less system\n");
> --
> 2.19.1
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] target/microblaze: Fix possible array out of bounds in mmu_write()
@ 2020-11-06 10:23 ` Edgar E. Iglesias
0 siblings, 0 replies; 8+ messages in thread
From: Edgar E. Iglesias @ 2020-11-06 10:23 UTC (permalink / raw)
To: AlexChen; +Cc: QEMU Trivial, qemu-devel, zhang.zhanghailiang
On Tue, Nov 03, 2020 at 03:46:02PM +0800, 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>
Reviewed-by: Edgar E. Iglesias <edgar.iglesias@xilinx.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]);
>
> if (cpu->cfg.mmu < 2 || !cpu->cfg.mmu_tlb_access) {
> qemu_log_mask(LOG_GUEST_ERROR, "MMU access on MMU-less system\n");
> --
> 2.19.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] target/microblaze: Fix possible array out of bounds in mmu_write()
2020-11-03 7:46 ` AlexChen
` (2 preceding siblings ...)
(?)
@ 2020-11-06 14:16 ` Philippe Mathieu-Daudé
2020-11-09 3:17 ` AlexChen
-1 siblings, 1 reply; 8+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-11-06 14:16 UTC (permalink / raw)
To: AlexChen, edgar.iglesias; +Cc: QEMU Trivial, qemu-devel, zhang.zhanghailiang
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...
>
> if (cpu->cfg.mmu < 2 || !cpu->cfg.mmu_tlb_access) {
> qemu_log_mask(LOG_GUEST_ERROR, "MMU access on MMU-less system\n");
>
^ permalink raw reply [flat|nested] 8+ messages in thread* Re: [PATCH] target/microblaze: Fix possible array out of bounds in mmu_write()
2020-11-06 14:16 ` Philippe Mathieu-Daudé
@ 2020-11-09 3:17 ` AlexChen
0 siblings, 0 replies; 8+ messages in thread
From: AlexChen @ 2020-11-09 3:17 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: edgar.iglesias, QEMU Trivial, qemu-devel, zhang.zhanghailiang
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] target/microblaze: Fix possible array out of bounds in mmu_write()
@ 2020-11-09 3:17 ` AlexChen
0 siblings, 0 replies; 8+ messages in thread
From: AlexChen @ 2020-11-09 3:17 UTC (permalink / raw)
To: Philippe Mathieu-Daudé
Cc: QEMU Trivial, edgar.iglesias, qemu-devel, zhang.zhanghailiang
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
^ permalink raw reply related [flat|nested] 8+ messages in thread