public inbox for kvm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] kvm: fix poison overwritten caused by using wrong xstate size
       [not found] <AANLkTikx35Sp0GR4c_REYGcZV4bMgS+T-1cugen45d58@mail.gmail.com>
@ 2010-08-13  7:19 ` Xiaotian Feng
  2010-08-13  7:56   ` Sheng Yang
                     ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Xiaotian Feng @ 2010-08-13  7:19 UTC (permalink / raw)
  To: x86, kvm
  Cc: linux-kernel, Xiaotian Feng, Thomas Gleixner, Ingo Molnar,
	H. Peter Anvin, Suresh Siddha, Brian Gerst, Avi Kivity,
	Robert Richter, Sheng Yang, Marcelo Tosatti, Gleb Natapov,
	Jan Kiszka

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;
-- 
1.7.2.1


^ permalink raw reply related	[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: 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-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

* 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

end of thread, other threads:[~2010-08-16  4:13 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <AANLkTikx35Sp0GR4c_REYGcZV4bMgS+T-1cugen45d58@mail.gmail.com>
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-16  4:12       ` H. Peter Anvin
2010-08-15 11:08   ` Avi Kivity

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox