* Re: [PATCH] kvm: fix poison overwritten caused by using wrong xstate size
2010-08-13 7:19 ` [PATCH] kvm: fix poison overwritten caused by using wrong xstate size Xiaotian Feng
@ 2010-08-13 7:56 ` Sheng Yang
2010-08-13 21:03 ` H. Peter Anvin
2010-08-15 11:08 ` Avi Kivity
2 siblings, 0 replies; 6+ messages in thread
From: Sheng Yang @ 2010-08-13 7:56 UTC (permalink / raw)
To: Xiaotian Feng
Cc: x86, kvm, linux-kernel, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Suresh Siddha, Brian Gerst, Avi Kivity,
Robert Richter, Marcelo Tosatti, Gleb Natapov, Jan Kiszka
On Friday 13 August 2010 15:19:11 Xiaotian Feng wrote:
> fpu.state is allocated from task_xstate_cachep, the size of
> task_xstate_cachep is xstate_size. xstate_size is set from cpuid
> instruction, which is often smaller than sizeof(struct xsave_struct). kvm
> is using sizeof(struct xsave_struct) to fill in/out fpu.state.xsave, as
> what we allocated for fpu.state is xstate_size, kernel will write out of
> memory and caused poison/redzone/padding overwritten warnings.
>
> Signed-off-by: Xiaotian Feng <dfeng@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Brian Gerst <brgerst@gmail.com>
> Cc: Avi Kivity <avi@redhat.com>
> Cc: Robert Richter <robert.richter@amd.com>
> Cc: Sheng Yang <sheng@linux.intel.com>
> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> Cc: Gleb Natapov <gleb@redhat.com>
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
Reviewed-by: Sheng Yang <sheng@linux.intel.com>
--
regards
Yang, Sheng
> ---
> arch/x86/kernel/i387.c | 1 +
> arch/x86/kvm/x86.c | 4 ++--
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
> index 1f11f5c..a46cb35 100644
> --- a/arch/x86/kernel/i387.c
> +++ b/arch/x86/kernel/i387.c
> @@ -40,6 +40,7 @@
>
> static unsigned int mxcsr_feature_mask __read_mostly = 0xffffffffu;
> unsigned int xstate_size;
> +EXPORT_SYMBOL_GPL(xstate_size);
> unsigned int sig_xstate_ia32_size = sizeof(struct _fpstate_ia32);
> static struct i387_fxsave_struct fx_scratch __cpuinitdata;
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 25f1907..3a09c62 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2387,7 +2387,7 @@ static void kvm_vcpu_ioctl_x86_get_xsave(struct
> kvm_vcpu *vcpu, if (cpu_has_xsave)
> memcpy(guest_xsave->region,
> &vcpu->arch.guest_fpu.state->xsave,
> - sizeof(struct xsave_struct));
> + xstate_size);
> else {
> memcpy(guest_xsave->region,
> &vcpu->arch.guest_fpu.state->fxsave,
> @@ -2405,7 +2405,7 @@ static int kvm_vcpu_ioctl_x86_set_xsave(struct
> kvm_vcpu *vcpu,
>
> if (cpu_has_xsave)
> memcpy(&vcpu->arch.guest_fpu.state->xsave,
> - guest_xsave->region, sizeof(struct xsave_struct));
> + guest_xsave->region, xstate_size);
> else {
> if (xstate_bv & ~XSTATE_FPSSE)
> return -EINVAL;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] kvm: fix poison overwritten caused by using wrong xstate size
2010-08-13 7:19 ` [PATCH] kvm: fix poison overwritten caused by using wrong xstate size Xiaotian Feng
2010-08-13 7:56 ` Sheng Yang
@ 2010-08-13 21:03 ` H. Peter Anvin
2010-08-15 11:05 ` Avi Kivity
2010-08-15 11:08 ` Avi Kivity
2 siblings, 1 reply; 6+ messages in thread
From: H. Peter Anvin @ 2010-08-13 21:03 UTC (permalink / raw)
To: Xiaotian Feng, avi Kivity
Cc: x86, kvm, linux-kernel, Thomas Gleixner, Ingo Molnar,
Suresh Siddha, Brian Gerst, Avi Kivity, Robert Richter,
Sheng Yang, Marcelo Tosatti, Gleb Natapov, Jan Kiszka
Avi, do you want to take this one or should I?
-hpa
On 08/13/2010 12:19 AM, Xiaotian Feng wrote:
> fpu.state is allocated from task_xstate_cachep, the size of task_xstate_cachep
> is xstate_size. xstate_size is set from cpuid instruction, which is often
> smaller than sizeof(struct xsave_struct). kvm is using sizeof(struct xsave_struct)
> to fill in/out fpu.state.xsave, as what we allocated for fpu.state is
> xstate_size, kernel will write out of memory and caused poison/redzone/padding
> overwritten warnings.
>
> Signed-off-by: Xiaotian Feng <dfeng@redhat.com>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: "H. Peter Anvin" <hpa@zytor.com>
> Cc: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Brian Gerst <brgerst@gmail.com>
> Cc: Avi Kivity <avi@redhat.com>
> Cc: Robert Richter <robert.richter@amd.com>
> Cc: Sheng Yang <sheng@linux.intel.com>
> Cc: Marcelo Tosatti <mtosatti@redhat.com>
> Cc: Gleb Natapov <gleb@redhat.com>
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> ---
> arch/x86/kernel/i387.c | 1 +
> arch/x86/kvm/x86.c | 4 ++--
> 2 files changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/kernel/i387.c b/arch/x86/kernel/i387.c
> index 1f11f5c..a46cb35 100644
> --- a/arch/x86/kernel/i387.c
> +++ b/arch/x86/kernel/i387.c
> @@ -40,6 +40,7 @@
>
> static unsigned int mxcsr_feature_mask __read_mostly = 0xffffffffu;
> unsigned int xstate_size;
> +EXPORT_SYMBOL_GPL(xstate_size);
> unsigned int sig_xstate_ia32_size = sizeof(struct _fpstate_ia32);
> static struct i387_fxsave_struct fx_scratch __cpuinitdata;
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 25f1907..3a09c62 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2387,7 +2387,7 @@ static void kvm_vcpu_ioctl_x86_get_xsave(struct kvm_vcpu *vcpu,
> if (cpu_has_xsave)
> memcpy(guest_xsave->region,
> &vcpu->arch.guest_fpu.state->xsave,
> - sizeof(struct xsave_struct));
> + xstate_size);
> else {
> memcpy(guest_xsave->region,
> &vcpu->arch.guest_fpu.state->fxsave,
> @@ -2405,7 +2405,7 @@ static int kvm_vcpu_ioctl_x86_set_xsave(struct kvm_vcpu *vcpu,
>
> if (cpu_has_xsave)
> memcpy(&vcpu->arch.guest_fpu.state->xsave,
> - guest_xsave->region, sizeof(struct xsave_struct));
> + guest_xsave->region, xstate_size);
> else {
> if (xstate_bv & ~XSTATE_FPSSE)
> return -EINVAL;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] kvm: fix poison overwritten caused by using wrong xstate size
2010-08-13 21:03 ` H. Peter Anvin
@ 2010-08-15 11:05 ` Avi Kivity
2010-08-16 4:12 ` H. Peter Anvin
0 siblings, 1 reply; 6+ messages in thread
From: Avi Kivity @ 2010-08-15 11:05 UTC (permalink / raw)
To: H. Peter Anvin
Cc: Xiaotian Feng, x86, kvm, linux-kernel, Thomas Gleixner,
Ingo Molnar, Suresh Siddha, Brian Gerst, Robert Richter,
Sheng Yang, Marcelo Tosatti, Gleb Natapov, Jan Kiszka
On 08/14/2010 12:03 AM, H. Peter Anvin wrote:
> Avi, do you want to take this one or should I?
I will, thanks.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kvm: fix poison overwritten caused by using wrong xstate size
2010-08-15 11:05 ` Avi Kivity
@ 2010-08-16 4:12 ` H. Peter Anvin
0 siblings, 0 replies; 6+ messages in thread
From: H. Peter Anvin @ 2010-08-16 4:12 UTC (permalink / raw)
To: Avi Kivity
Cc: Xiaotian Feng, x86, kvm, linux-kernel, Thomas Gleixner,
Ingo Molnar, Suresh Siddha, Brian Gerst, Robert Richter,
Sheng Yang, Marcelo Tosatti, Gleb Natapov, Jan Kiszka
Feel free to add my ack.
"Avi Kivity" <avi@redhat.com> wrote:
> On 08/14/2010 12:03 AM, H. Peter Anvin wrote:
>> Avi, do you want to take this one or should I?
>
>I will, thanks.
>
>--
>error compiling committee.c: too many arguments to function
>
--
Sent from my mobile phone. Please pardon any lack of formatting.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] kvm: fix poison overwritten caused by using wrong xstate size
2010-08-13 7:19 ` [PATCH] kvm: fix poison overwritten caused by using wrong xstate size Xiaotian Feng
2010-08-13 7:56 ` Sheng Yang
2010-08-13 21:03 ` H. Peter Anvin
@ 2010-08-15 11:08 ` Avi Kivity
2 siblings, 0 replies; 6+ messages in thread
From: Avi Kivity @ 2010-08-15 11:08 UTC (permalink / raw)
To: Xiaotian Feng
Cc: x86, kvm, linux-kernel, Thomas Gleixner, Ingo Molnar,
H. Peter Anvin, Suresh Siddha, Brian Gerst, Robert Richter,
Sheng Yang, Marcelo Tosatti, Gleb Natapov, Jan Kiszka
On 08/13/2010 10:19 AM, Xiaotian Feng wrote:
> fpu.state is allocated from task_xstate_cachep, the size of task_xstate_cachep
> is xstate_size. xstate_size is set from cpuid instruction, which is often
> smaller than sizeof(struct xsave_struct). kvm is using sizeof(struct xsave_struct)
> to fill in/out fpu.state.xsave, as what we allocated for fpu.state is
> xstate_size, kernel will write out of memory and caused poison/redzone/padding
> overwritten warnings.
Thanks, applied.
--
error compiling committee.c: too many arguments to function
^ permalink raw reply [flat|nested] 6+ messages in thread