* [PATCH] target/i386: avoid theoretical leak on MCE injection
@ 2020-10-06 7:48 Paolo Bonzini
2020-11-12 15:53 ` Peter Maydell
0 siblings, 1 reply; 2+ messages in thread
From: Paolo Bonzini @ 2020-10-06 7:48 UTC (permalink / raw)
To: qemu-devel
g_strdup_printf is used twice to write to the same variable, which
can theoretically cause a leak. In practice, it is extremely
unlikely that a guest is seeing a recursive MCE and has disabled
CR4.MCE between the first and the second error, but we can fix it
and we can also make a slight improvement on the logic: CR4.MCE=0
causes a triple fault even for a non-recursive machine check, so
let's place its test first.
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
target/i386/helper.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/target/i386/helper.c b/target/i386/helper.c
index 32fa21a7bb..f64379367d 100644
--- a/target/i386/helper.c
+++ b/target/i386/helper.c
@@ -908,16 +908,14 @@ static void do_inject_x86_mce(CPUState *cs, run_on_cpu_data data)
return;
}
- if (recursive) {
- need_reset = true;
- msg = g_strdup_printf("CPU %d: Previous MCE still in progress, "
- "raising triple fault", cs->cpu_index);
- }
-
if (!(cenv->cr[4] & CR4_MCE_MASK)) {
need_reset = true;
msg = g_strdup_printf("CPU %d: MCE capability is not enabled, "
"raising triple fault", cs->cpu_index);
+ } else if (recursive) {
+ need_reset = true;
+ msg = g_strdup_printf("CPU %d: Previous MCE still in progress, "
+ "raising triple fault", cs->cpu_index);
}
if (need_reset) {
--
2.26.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] target/i386: avoid theoretical leak on MCE injection
2020-10-06 7:48 [PATCH] target/i386: avoid theoretical leak on MCE injection Paolo Bonzini
@ 2020-11-12 15:53 ` Peter Maydell
0 siblings, 0 replies; 2+ messages in thread
From: Peter Maydell @ 2020-11-12 15:53 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: QEMU Developers
On Tue, 6 Oct 2020 at 08:54, Paolo Bonzini <pbonzini@redhat.com> wrote:
>
> g_strdup_printf is used twice to write to the same variable, which
> can theoretically cause a leak. In practice, it is extremely
> unlikely that a guest is seeing a recursive MCE and has disabled
> CR4.MCE between the first and the second error, but we can fix it
> and we can also make a slight improvement on the logic: CR4.MCE=0
> causes a triple fault even for a non-recursive machine check, so
> let's place its test first.
>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> ---
> target/i386/helper.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/target/i386/helper.c b/target/i386/helper.c
> index 32fa21a7bb..f64379367d 100644
> --- a/target/i386/helper.c
> +++ b/target/i386/helper.c
> @@ -908,16 +908,14 @@ static void do_inject_x86_mce(CPUState *cs, run_on_cpu_data data)
> return;
> }
>
> - if (recursive) {
> - need_reset = true;
> - msg = g_strdup_printf("CPU %d: Previous MCE still in progress, "
> - "raising triple fault", cs->cpu_index);
> - }
> -
> if (!(cenv->cr[4] & CR4_MCE_MASK)) {
> need_reset = true;
> msg = g_strdup_printf("CPU %d: MCE capability is not enabled, "
> "raising triple fault", cs->cpu_index);
> + } else if (recursive) {
> + need_reset = true;
> + msg = g_strdup_printf("CPU %d: Previous MCE still in progress, "
> + "raising triple fault", cs->cpu_index);
> }
>
> if (need_reset) {
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Might be nice to have this in 5.2, given it is a coverity issue fix?
thanks
-- PMM
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-11-12 15:55 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-06 7:48 [PATCH] target/i386: avoid theoretical leak on MCE injection Paolo Bonzini
2020-11-12 15:53 ` Peter Maydell
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.