* [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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox