[PATCH 0/2] Fix xsave/xcr save/restore memory leak

All of lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH 0/2] Fix xsave/xcr save/restore memory leak
@ 2010-06-20 13:14 Avi Kivity
  2010-06-20 13:14 ` [PATCH 1/2] KVM: Fix xsave and xcr " Avi Kivity
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Avi Kivity @ 2010-06-20 13:14 UTC (permalink / raw)
  To: Marcelo Tosatti, Sheng Yang; +Cc: kvm

There's a small leak in xsave/xcr save/restore that rapidly drains all memory
during Windows XP install without FlexPriority, since that triggers qemu
register reload frequently.

Avi Kivity (2):
  KVM: Fix xsave and xcr save/restore memory leak
  KVM: Consolidate load/save temporary buffer allocation and freeing

 arch/x86/kvm/x86.c |   66 +++++++++++++++++++++++++--------------------------
 1 files changed, 32 insertions(+), 34 deletions(-)


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] KVM: Fix xsave and xcr save/restore memory leak
  2010-06-20 13:14 [PATCH 0/2] Fix xsave/xcr save/restore memory leak Avi Kivity
@ 2010-06-20 13:14 ` Avi Kivity
  2010-06-20 13:14 ` [PATCH 2/2] KVM: Consolidate load/save temporary buffer allocation and freeing Avi Kivity
  2010-06-21  1:09 ` [PATCH 0/2] Fix xsave/xcr save/restore memory leak Sheng Yang
  2 siblings, 0 replies; 4+ messages in thread
From: Avi Kivity @ 2010-06-20 13:14 UTC (permalink / raw)
  To: Marcelo Tosatti, Sheng Yang; +Cc: kvm

We allocate temporary kernel buffers for these structures, but never free them.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/x86.c |   12 ++++--------
 1 files changed, 4 insertions(+), 8 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d3d008e..d513e57 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2437,6 +2437,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 	void __user *argp = (void __user *)arg;
 	int r;
 	struct kvm_lapic_state *lapic = NULL;
+	struct kvm_xsave *xsave = NULL;
+	struct kvm_xcrs *xcrs = NULL;
 
 	switch (ioctl) {
 	case KVM_GET_LAPIC: {
@@ -2632,8 +2634,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		break;
 	}
 	case KVM_GET_XSAVE: {
-		struct kvm_xsave *xsave;
-
 		xsave = kzalloc(sizeof(struct kvm_xsave), GFP_KERNEL);
 		r = -ENOMEM;
 		if (!xsave)
@@ -2648,8 +2648,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		break;
 	}
 	case KVM_SET_XSAVE: {
-		struct kvm_xsave *xsave;
-
 		xsave = kzalloc(sizeof(struct kvm_xsave), GFP_KERNEL);
 		r = -ENOMEM;
 		if (!xsave)
@@ -2663,8 +2661,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		break;
 	}
 	case KVM_GET_XCRS: {
-		struct kvm_xcrs *xcrs;
-
 		xcrs = kzalloc(sizeof(struct kvm_xcrs), GFP_KERNEL);
 		r = -ENOMEM;
 		if (!xcrs)
@@ -2680,8 +2676,6 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		break;
 	}
 	case KVM_SET_XCRS: {
-		struct kvm_xcrs *xcrs;
-
 		xcrs = kzalloc(sizeof(struct kvm_xcrs), GFP_KERNEL);
 		r = -ENOMEM;
 		if (!xcrs)
@@ -2700,6 +2694,8 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 	}
 out:
 	kfree(lapic);
+	kfree(xsave);
+	kfree(xcrs);
 	return r;
 }
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] KVM: Consolidate load/save temporary buffer allocation and freeing
  2010-06-20 13:14 [PATCH 0/2] Fix xsave/xcr save/restore memory leak Avi Kivity
  2010-06-20 13:14 ` [PATCH 1/2] KVM: Fix xsave and xcr " Avi Kivity
@ 2010-06-20 13:14 ` Avi Kivity
  2010-06-21  1:09 ` [PATCH 0/2] Fix xsave/xcr save/restore memory leak Sheng Yang
  2 siblings, 0 replies; 4+ messages in thread
From: Avi Kivity @ 2010-06-20 13:14 UTC (permalink / raw)
  To: Marcelo Tosatti, Sheng Yang; +Cc: kvm

Instead of three temporary variables and three free calls, have one temporary
variable (with four names) and one free call.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 arch/x86/kvm/x86.c |   64 ++++++++++++++++++++++++++-------------------------
 1 files changed, 33 insertions(+), 31 deletions(-)

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index d513e57..33156a3 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2436,25 +2436,29 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 	struct kvm_vcpu *vcpu = filp->private_data;
 	void __user *argp = (void __user *)arg;
 	int r;
-	struct kvm_lapic_state *lapic = NULL;
-	struct kvm_xsave *xsave = NULL;
-	struct kvm_xcrs *xcrs = NULL;
+	union {
+		struct kvm_lapic_state *lapic;
+		struct kvm_xsave *xsave;
+		struct kvm_xcrs *xcrs;
+		void *buffer;
+	} u;
 
+	u.buffer = NULL;
 	switch (ioctl) {
 	case KVM_GET_LAPIC: {
 		r = -EINVAL;
 		if (!vcpu->arch.apic)
 			goto out;
-		lapic = kzalloc(sizeof(struct kvm_lapic_state), GFP_KERNEL);
+		u.lapic = kzalloc(sizeof(struct kvm_lapic_state), GFP_KERNEL);
 
 		r = -ENOMEM;
-		if (!lapic)
+		if (!u.lapic)
 			goto out;
-		r = kvm_vcpu_ioctl_get_lapic(vcpu, lapic);
+		r = kvm_vcpu_ioctl_get_lapic(vcpu, u.lapic);
 		if (r)
 			goto out;
 		r = -EFAULT;
-		if (copy_to_user(argp, lapic, sizeof(struct kvm_lapic_state)))
+		if (copy_to_user(argp, u.lapic, sizeof(struct kvm_lapic_state)))
 			goto out;
 		r = 0;
 		break;
@@ -2463,14 +2467,14 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		r = -EINVAL;
 		if (!vcpu->arch.apic)
 			goto out;
-		lapic = kmalloc(sizeof(struct kvm_lapic_state), GFP_KERNEL);
+		u.lapic = kmalloc(sizeof(struct kvm_lapic_state), GFP_KERNEL);
 		r = -ENOMEM;
-		if (!lapic)
+		if (!u.lapic)
 			goto out;
 		r = -EFAULT;
-		if (copy_from_user(lapic, argp, sizeof(struct kvm_lapic_state)))
+		if (copy_from_user(u.lapic, argp, sizeof(struct kvm_lapic_state)))
 			goto out;
-		r = kvm_vcpu_ioctl_set_lapic(vcpu, lapic);
+		r = kvm_vcpu_ioctl_set_lapic(vcpu, u.lapic);
 		if (r)
 			goto out;
 		r = 0;
@@ -2634,68 +2638,66 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		break;
 	}
 	case KVM_GET_XSAVE: {
-		xsave = kzalloc(sizeof(struct kvm_xsave), GFP_KERNEL);
+		u.xsave = kzalloc(sizeof(struct kvm_xsave), GFP_KERNEL);
 		r = -ENOMEM;
-		if (!xsave)
+		if (!u.xsave)
 			break;
 
-		kvm_vcpu_ioctl_x86_get_xsave(vcpu, xsave);
+		kvm_vcpu_ioctl_x86_get_xsave(vcpu, u.xsave);
 
 		r = -EFAULT;
-		if (copy_to_user(argp, xsave, sizeof(struct kvm_xsave)))
+		if (copy_to_user(argp, u.xsave, sizeof(struct kvm_xsave)))
 			break;
 		r = 0;
 		break;
 	}
 	case KVM_SET_XSAVE: {
-		xsave = kzalloc(sizeof(struct kvm_xsave), GFP_KERNEL);
+		u.xsave = kzalloc(sizeof(struct kvm_xsave), GFP_KERNEL);
 		r = -ENOMEM;
-		if (!xsave)
+		if (!u.xsave)
 			break;
 
 		r = -EFAULT;
-		if (copy_from_user(xsave, argp, sizeof(struct kvm_xsave)))
+		if (copy_from_user(u.xsave, argp, sizeof(struct kvm_xsave)))
 			break;
 
-		r = kvm_vcpu_ioctl_x86_set_xsave(vcpu, xsave);
+		r = kvm_vcpu_ioctl_x86_set_xsave(vcpu, u.xsave);
 		break;
 	}
 	case KVM_GET_XCRS: {
-		xcrs = kzalloc(sizeof(struct kvm_xcrs), GFP_KERNEL);
+		u.xcrs = kzalloc(sizeof(struct kvm_xcrs), GFP_KERNEL);
 		r = -ENOMEM;
-		if (!xcrs)
+		if (!u.xcrs)
 			break;
 
-		kvm_vcpu_ioctl_x86_get_xcrs(vcpu, xcrs);
+		kvm_vcpu_ioctl_x86_get_xcrs(vcpu, u.xcrs);
 
 		r = -EFAULT;
-		if (copy_to_user(argp, xcrs,
+		if (copy_to_user(argp, u.xcrs,
 				 sizeof(struct kvm_xcrs)))
 			break;
 		r = 0;
 		break;
 	}
 	case KVM_SET_XCRS: {
-		xcrs = kzalloc(sizeof(struct kvm_xcrs), GFP_KERNEL);
+		u.xcrs = kzalloc(sizeof(struct kvm_xcrs), GFP_KERNEL);
 		r = -ENOMEM;
-		if (!xcrs)
+		if (!u.xcrs)
 			break;
 
 		r = -EFAULT;
-		if (copy_from_user(xcrs, argp,
+		if (copy_from_user(u.xcrs, argp,
 				   sizeof(struct kvm_xcrs)))
 			break;
 
-		r = kvm_vcpu_ioctl_x86_set_xcrs(vcpu, xcrs);
+		r = kvm_vcpu_ioctl_x86_set_xcrs(vcpu, u.xcrs);
 		break;
 	}
 	default:
 		r = -EINVAL;
 	}
 out:
-	kfree(lapic);
-	kfree(xsave);
-	kfree(xcrs);
+	kfree(u.buffer);
 	return r;
 }
 
-- 
1.7.1


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 0/2] Fix xsave/xcr save/restore memory leak
  2010-06-20 13:14 [PATCH 0/2] Fix xsave/xcr save/restore memory leak Avi Kivity
  2010-06-20 13:14 ` [PATCH 1/2] KVM: Fix xsave and xcr " Avi Kivity
  2010-06-20 13:14 ` [PATCH 2/2] KVM: Consolidate load/save temporary buffer allocation and freeing Avi Kivity
@ 2010-06-21  1:09 ` Sheng Yang
  2 siblings, 0 replies; 4+ messages in thread
From: Sheng Yang @ 2010-06-21  1:09 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm

On Sunday 20 June 2010 21:14:11 Avi Kivity wrote:
> There's a small leak in xsave/xcr save/restore that rapidly drains all
> memory during Windows XP install without FlexPriority, since that triggers
> qemu register reload frequently.

Oops...

Would be more careful next time...

--
regards
Yang, Sheng

> 
> Avi Kivity (2):
>   KVM: Fix xsave and xcr save/restore memory leak
>   KVM: Consolidate load/save temporary buffer allocation and freeing
> 
>  arch/x86/kvm/x86.c |   66
> +++++++++++++++++++++++++-------------------------- 1 files changed, 32
> insertions(+), 34 deletions(-)

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2010-06-21  1:09 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-20 13:14 [PATCH 0/2] Fix xsave/xcr save/restore memory leak Avi Kivity
2010-06-20 13:14 ` [PATCH 1/2] KVM: Fix xsave and xcr " Avi Kivity
2010-06-20 13:14 ` [PATCH 2/2] KVM: Consolidate load/save temporary buffer allocation and freeing Avi Kivity
2010-06-21  1:09 ` [PATCH 0/2] Fix xsave/xcr save/restore memory leak Sheng Yang

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.