Skip to content

Commit 85cd39a

Browse files
committed
KVM: Do not leak memory for duplicate debugfs directories
KVM creates a debugfs directory for each VM in order to store statistics about the virtual machine. The directory name is built from the process pid and a VM fd. While generally unique, it is possible to keep a file descriptor alive in a way that causes duplicate directories, which manifests as these messages: [ 471.846235] debugfs: Directory '20245-4' with parent 'kvm' already present! Even though this should not happen in practice, it is more or less expected in the case of KVM for testcases that call KVM_CREATE_VM and close the resulting file descriptor repeatedly and in parallel. When this happens, debugfs_create_dir() returns an error but kvm_create_vm_debugfs() goes on to allocate stat data structs which are later leaked. The slow memory leak was spotted by syzkaller, where it caused OOM reports. Since the issue only affects debugfs, do a lookup before calling debugfs_create_dir, so that the message is downgraded and rate-limited. While at it, ensure kvm->debugfs_dentry is NULL rather than an error if it is not created. This fixes kvm_destroy_vm_debugfs, which was not checking IS_ERR_OR_NULL correctly. Cc: [email protected] Fixes: 536a6f8 ("KVM: Create debugfs dir and stat files for each VM") Reported-by: Alexey Kardashevskiy <[email protected]> Suggested-by: Greg Kroah-Hartman <[email protected]> Acked-by: Greg Kroah-Hartman <[email protected]> Signed-off-by: Paolo Bonzini <[email protected]>
1 parent 2476b5a commit 85cd39a

File tree

1 file changed

+16
-2
lines changed

1 file changed

+16
-2
lines changed

virt/kvm/kvm_main.c

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -892,6 +892,8 @@ static void kvm_destroy_vm_debugfs(struct kvm *kvm)
892892

893893
static int kvm_create_vm_debugfs(struct kvm *kvm, int fd)
894894
{
895+
static DEFINE_MUTEX(kvm_debugfs_lock);
896+
struct dentry *dent;
895897
char dir_name[ITOA_MAX_LEN * 2];
896898
struct kvm_stat_data *stat_data;
897899
const struct _kvm_stats_desc *pdesc;
@@ -903,8 +905,20 @@ static int kvm_create_vm_debugfs(struct kvm *kvm, int fd)
903905
return 0;
904906

905907
snprintf(dir_name, sizeof(dir_name), "%d-%d", task_pid_nr(current), fd);
906-
kvm->debugfs_dentry = debugfs_create_dir(dir_name, kvm_debugfs_dir);
908+
mutex_lock(&kvm_debugfs_lock);
909+
dent = debugfs_lookup(dir_name, kvm_debugfs_dir);
910+
if (dent) {
911+
pr_warn_ratelimited("KVM: debugfs: duplicate directory %s\n", dir_name);
912+
dput(dent);
913+
mutex_unlock(&kvm_debugfs_lock);
914+
return 0;
915+
}
916+
dent = debugfs_create_dir(dir_name, kvm_debugfs_dir);
917+
mutex_unlock(&kvm_debugfs_lock);
918+
if (IS_ERR(dent))
919+
return 0;
907920

921+
kvm->debugfs_dentry = dent;
908922
kvm->debugfs_stat_data = kcalloc(kvm_debugfs_num_entries,
909923
sizeof(*kvm->debugfs_stat_data),
910924
GFP_KERNEL_ACCOUNT);
@@ -5201,7 +5215,7 @@ static void kvm_uevent_notify_change(unsigned int type, struct kvm *kvm)
52015215
}
52025216
add_uevent_var(env, "PID=%d", kvm->userspace_pid);
52035217

5204-
if (!IS_ERR_OR_NULL(kvm->debugfs_dentry)) {
5218+
if (kvm->debugfs_dentry) {
52055219
char *tmp, *p = kmalloc(PATH_MAX, GFP_KERNEL_ACCOUNT);
52065220

52075221
if (p) {

0 commit comments

Comments
 (0)