* [PATCH for-7.2] target/i386: Always completely initialize TranslateFault
@ 2022-12-01 7:45 Richard Henderson
2022-12-01 7:47 ` Richard Henderson
2022-12-01 8:51 ` Paolo Bonzini
0 siblings, 2 replies; 3+ messages in thread
From: Richard Henderson @ 2022-12-01 7:45 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, eduardo, Daniel Hoffman
In get_physical_address, the canonical address check failed to
set TranslateFault.stage2, which resulted in an uninitialized
read from the struct when reporting the fault in x86_cpu_tlb_fill.
Adjust all error paths to use structure assignment so that the
entire struct is always initialized.
Reported-by: Daniel Hoffman <dhoff749@gmail.com>
Fixes: 9bbcf372193a ("target/i386: Reorg GET_HPHYS")
Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
---
target/i386/tcg/sysemu/excp_helper.c | 34 ++++++++++++++++------------
1 file changed, 19 insertions(+), 15 deletions(-)
diff --git a/target/i386/tcg/sysemu/excp_helper.c b/target/i386/tcg/sysemu/excp_helper.c
index 405a5d414a..55bd1194d3 100644
--- a/target/i386/tcg/sysemu/excp_helper.c
+++ b/target/i386/tcg/sysemu/excp_helper.c
@@ -71,10 +71,11 @@ static bool ptw_translate(PTETranslate *inout, hwaddr addr)
TranslateFault *err = inout->err;
assert(inout->ptw_idx == MMU_NESTED_IDX);
- err->exception_index = 0; /* unused */
- err->error_code = inout->env->error_code;
- err->cr2 = addr;
- err->stage2 = S2_GPT;
+ *err = (TranslateFault){
+ .error_code = inout->env->error_code,
+ .cr2 = addr,
+ .stage2 = S2_GPT,
+ };
return false;
}
return true;
@@ -431,10 +432,11 @@ do_check_protect_pse36:
MMU_NESTED_IDX, true,
&pte_trans.haddr, &full, 0);
if (unlikely(flags & TLB_INVALID_MASK)) {
- err->exception_index = 0; /* unused */
- err->error_code = env->error_code;
- err->cr2 = paddr;
- err->stage2 = S2_GPA;
+ *err = (TranslateFault){
+ .error_code = env->error_code,
+ .cr2 = paddr,
+ .stage2 = S2_GPA,
+ };
return false;
}
@@ -494,10 +496,11 @@ do_check_protect_pse36:
}
break;
}
- err->exception_index = EXCP0E_PAGE;
- err->error_code = error_code;
- err->cr2 = addr;
- err->stage2 = S2_NONE;
+ *err = (TranslateFault){
+ .exception_index = EXCP0E_PAGE,
+ .error_code = error_code,
+ .cr2 = addr,
+ };
return false;
}
@@ -564,9 +567,10 @@ static bool get_physical_address(CPUX86State *env, vaddr addr,
int shift = in.pg_mode & PG_MODE_LA57 ? 56 : 47;
int64_t sext = (int64_t)addr >> shift;
if (sext != 0 && sext != -1) {
- err->exception_index = EXCP0D_GPF;
- err->error_code = 0;
- err->cr2 = addr;
+ *err = (TranslateFault){
+ .exception_index = EXCP0D_GPF,
+ .cr2 = addr,
+ };
return false;
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH for-7.2] target/i386: Always completely initialize TranslateFault
2022-12-01 7:45 [PATCH for-7.2] target/i386: Always completely initialize TranslateFault Richard Henderson
@ 2022-12-01 7:47 ` Richard Henderson
2022-12-01 8:51 ` Paolo Bonzini
1 sibling, 0 replies; 3+ messages in thread
From: Richard Henderson @ 2022-12-01 7:47 UTC (permalink / raw)
To: qemu-devel; +Cc: pbonzini, eduardo, Daniel Hoffman
On 11/30/22 23:45, Richard Henderson wrote:
> In get_physical_address, the canonical address check failed to
> set TranslateFault.stage2, which resulted in an uninitialized
> read from the struct when reporting the fault in x86_cpu_tlb_fill.
>
> Adjust all error paths to use structure assignment so that the
> entire struct is always initialized.
>
> Reported-by: Daniel Hoffman <dhoff749@gmail.com>
> Fixes: 9bbcf372193a ("target/i386: Reorg GET_HPHYS")
> Signed-off-by: Richard Henderson <richard.henderson@linaro.org>
> ---
> target/i386/tcg/sysemu/excp_helper.c | 34 ++++++++++++++++------------
> 1 file changed, 19 insertions(+), 15 deletions(-)
Ah hah, I just found the issue filed for this:
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1324
r~
>
> diff --git a/target/i386/tcg/sysemu/excp_helper.c b/target/i386/tcg/sysemu/excp_helper.c
> index 405a5d414a..55bd1194d3 100644
> --- a/target/i386/tcg/sysemu/excp_helper.c
> +++ b/target/i386/tcg/sysemu/excp_helper.c
> @@ -71,10 +71,11 @@ static bool ptw_translate(PTETranslate *inout, hwaddr addr)
> TranslateFault *err = inout->err;
>
> assert(inout->ptw_idx == MMU_NESTED_IDX);
> - err->exception_index = 0; /* unused */
> - err->error_code = inout->env->error_code;
> - err->cr2 = addr;
> - err->stage2 = S2_GPT;
> + *err = (TranslateFault){
> + .error_code = inout->env->error_code,
> + .cr2 = addr,
> + .stage2 = S2_GPT,
> + };
> return false;
> }
> return true;
> @@ -431,10 +432,11 @@ do_check_protect_pse36:
> MMU_NESTED_IDX, true,
> &pte_trans.haddr, &full, 0);
> if (unlikely(flags & TLB_INVALID_MASK)) {
> - err->exception_index = 0; /* unused */
> - err->error_code = env->error_code;
> - err->cr2 = paddr;
> - err->stage2 = S2_GPA;
> + *err = (TranslateFault){
> + .error_code = env->error_code,
> + .cr2 = paddr,
> + .stage2 = S2_GPA,
> + };
> return false;
> }
>
> @@ -494,10 +496,11 @@ do_check_protect_pse36:
> }
> break;
> }
> - err->exception_index = EXCP0E_PAGE;
> - err->error_code = error_code;
> - err->cr2 = addr;
> - err->stage2 = S2_NONE;
> + *err = (TranslateFault){
> + .exception_index = EXCP0E_PAGE,
> + .error_code = error_code,
> + .cr2 = addr,
> + };
> return false;
> }
>
> @@ -564,9 +567,10 @@ static bool get_physical_address(CPUX86State *env, vaddr addr,
> int shift = in.pg_mode & PG_MODE_LA57 ? 56 : 47;
> int64_t sext = (int64_t)addr >> shift;
> if (sext != 0 && sext != -1) {
> - err->exception_index = EXCP0D_GPF;
> - err->error_code = 0;
> - err->cr2 = addr;
> + *err = (TranslateFault){
> + .exception_index = EXCP0D_GPF,
> + .cr2 = addr,
> + };
> return false;
> }
> }
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH for-7.2] target/i386: Always completely initialize TranslateFault
2022-12-01 7:45 [PATCH for-7.2] target/i386: Always completely initialize TranslateFault Richard Henderson
2022-12-01 7:47 ` Richard Henderson
@ 2022-12-01 8:51 ` Paolo Bonzini
1 sibling, 0 replies; 3+ messages in thread
From: Paolo Bonzini @ 2022-12-01 8:51 UTC (permalink / raw)
To: Richard Henderson; +Cc: qemu-devel, pbonzini, eduardo, Daniel Hoffman
Queued, thanks.
Paolo
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-12-01 8:53 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-01 7:45 [PATCH for-7.2] target/i386: Always completely initialize TranslateFault Richard Henderson
2022-12-01 7:47 ` Richard Henderson
2022-12-01 8:51 ` Paolo Bonzini
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.