diff mbox series

KVM: Fix OOM vulnerability caused by continuously creating devices

Message ID 20220108164948.42112-1-wang.yi59@zte.com.cn (mailing list archive)
State New, archived
Headers show
Series KVM: Fix OOM vulnerability caused by continuously creating devices | expand

Commit Message

Yi Wang Jan. 8, 2022, 4:49 p.m. UTC 
From: ZhaoQiang <zhao.qiang11@zte.com.cn>

When processing the ioctl request for creating a device in the
kvm_vm_ioctl()function,the branch did not reclaim the successfully
created device,which caused memory leak.

Signed-off-by: ZhaoQiang <zhao.qiang11@zte.com.cn>
Signed-off-by: Yi Wang <wang.yi59@zte.com.cn>
---
 virt/kvm/kvm_main.c | 39 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

Comments

Sean Christopherson Jan. 10, 2022, 4:12 p.m. UTC | #1 
On Sun, Jan 09, 2022, Yi Wang wrote:
> From: ZhaoQiang <zhao.qiang11@zte.com.cn>
> 
> When processing the ioctl request for creating a device in the
> kvm_vm_ioctl()function,the branch did not reclaim the successfully
> created device,which caused memory leak.

It's not a memory leak, kvm_destroy_vm() => kvm_destroy_devices() will free all
devices. anon_inode_getfd() installes the devices fd, so the device's fd and its
reference to KVM will be put when the process exits.  Am I missing something?
diff mbox series

Patch

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 72c4e6b39389..f4fbc935faea 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -52,6 +52,7 @@ 
 #include <linux/lockdep.h>
 #include <linux/kthread.h>
 #include <linux/suspend.h>
+#include <linux/syscalls.h>
 
 #include <asm/processor.h>
 #include <asm/ioctl.h>
@@ -4092,6 +4093,40 @@  static int kvm_ioctl_create_device(struct kvm *kvm,
 	return 0;
 }
 
+static int kvm_ioctl_destroy_device(struct kvm *kvm,
+				    struct kvm_create_device *cd)
+{
+	struct kvm_device_ops *ops = NULL;
+	struct kvm_device *dev;
+	struct file *file;
+	int type;
+
+	if (cd->type >= ARRAY_SIZE(kvm_device_ops_table))
+		return -ENODEV;
+
+	type = array_index_nospec(cd->type, ARRAY_SIZE(kvm_device_ops_table));
+	ops = kvm_device_ops_table[type];
+	if (ops == NULL)
+		return -ENODEV;
+
+	file = fget(cd->fd);
+	if (!file)
+		return -ENODEV;
+
+	dev = file->private_data;
+	if (!dev)
+		return -ENODEV;
+
+	kvm_put_kvm(kvm);
+	mutex_lock(&kvm->lock);
+	list_del(&device->vm_node);
+	mutex_unlock(&kvm->lock);
+	ops->destroy(dev);
+	ksys_close(cd->fd);
+
+	return 0;
+}
+
 static long kvm_vm_ioctl_check_extension_generic(struct kvm *kvm, long arg)
 {
 	switch (arg) {
@@ -4448,8 +4483,10 @@  static long kvm_vm_ioctl(struct file *filp,
 			goto out;
 
 		r = -EFAULT;
-		if (copy_to_user(argp, &cd, sizeof(cd)))
+		if (copy_to_user(argp, &cd, sizeof(cd))) {
+			kvm_ioctl_destroy_device(kvm, &cd);
 			goto out;
+		}
 
 		r = 0;
 		break;