All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch 1/2] KVM: fix error handling in kvm_create_vm_debugfs()
@ 2015-11-19 12:40 ` Dan Carpenter
  0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2015-11-19 12:40 UTC (permalink / raw)
  To: Gleb Natapov, Janosch Frank; +Cc: Paolo Bonzini, kvm, kernel-janitors

The "goto out_err" is buggy because we forgot to set the return code.

The other issue is that if the kmalloc() fails, we should remove the
debugfs directory before returning.

Fixes: 7805f53a85ec ('KVM: Create debugfs dir and stat files for each VM')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1ee2f73..f62621f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -571,12 +571,14 @@ static int kvm_create_vm_debugfs(struct kvm *kvm)
 	snprintf(dir_name, sizeof(dir_name), "%d", current->pid);
 	kvm->debugfs_dentry = debugfs_create_dir(dir_name, kvm_debugfs_dir);
 	if (!kvm->debugfs_dentry)
-		goto out_err;
+		return -ENOMEM;
 
 	kvm->debugfs_data = kmalloc(sizeof(*kvm->debugfs_data) *
 				    kvm_debugfs_num_entries, GFP_KERNEL);
-	if (!kvm->debugfs_data)
-		return -ENOMEM;
+	if (!kvm->debugfs_data) {
+		r = -ENOMEM;
+		goto err_remove_dir;
+	}
 
 	for (p = debugfs_entries; p->name; p++, i++) {
 		stat_data = kzalloc(sizeof(*stat_data), GFP_KERNEL);
@@ -600,13 +602,15 @@ static int kvm_create_vm_debugfs(struct kvm *kvm)
 	return r;
 
 out_err_clean:
-	debugfs_remove_recursive(kvm->debugfs_dentry);
 	kfree(stat_data);
 	for (i--; i >= 0; i--)
 		kfree(kvm->debugfs_data[i]);
 
 	kfree(kvm->debugfs_data);
-out_err:
+
+err_remove_dir:
+	debugfs_remove_recursive(kvm->debugfs_dentry);
+
 	return r;
 }
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* [patch 1/2] KVM: fix error handling in kvm_create_vm_debugfs()
@ 2015-11-19 12:40 ` Dan Carpenter
  0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2015-11-19 12:40 UTC (permalink / raw)
  To: Gleb Natapov, Janosch Frank; +Cc: Paolo Bonzini, kvm, kernel-janitors

The "goto out_err" is buggy because we forgot to set the return code.

The other issue is that if the kmalloc() fails, we should remove the
debugfs directory before returning.

Fixes: 7805f53a85ec ('KVM: Create debugfs dir and stat files for each VM')
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 1ee2f73..f62621f 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -571,12 +571,14 @@ static int kvm_create_vm_debugfs(struct kvm *kvm)
 	snprintf(dir_name, sizeof(dir_name), "%d", current->pid);
 	kvm->debugfs_dentry = debugfs_create_dir(dir_name, kvm_debugfs_dir);
 	if (!kvm->debugfs_dentry)
-		goto out_err;
+		return -ENOMEM;
 
 	kvm->debugfs_data = kmalloc(sizeof(*kvm->debugfs_data) *
 				    kvm_debugfs_num_entries, GFP_KERNEL);
-	if (!kvm->debugfs_data)
-		return -ENOMEM;
+	if (!kvm->debugfs_data) {
+		r = -ENOMEM;
+		goto err_remove_dir;
+	}
 
 	for (p = debugfs_entries; p->name; p++, i++) {
 		stat_data = kzalloc(sizeof(*stat_data), GFP_KERNEL);
@@ -600,13 +602,15 @@ static int kvm_create_vm_debugfs(struct kvm *kvm)
 	return r;
 
 out_err_clean:
-	debugfs_remove_recursive(kvm->debugfs_dentry);
 	kfree(stat_data);
 	for (i--; i >= 0; i--)
 		kfree(kvm->debugfs_data[i]);
 
 	kfree(kvm->debugfs_data);
-out_err:
+
+err_remove_dir:
+	debugfs_remove_recursive(kvm->debugfs_dentry);
+
 	return r;
 }
 

^ permalink raw reply related	[flat|nested] 6+ messages in thread
* Re: [patch 1/2] KVM: fix error handling in kvm_create_vm_debugfs()
  2015-11-19 12:40 ` Dan Carpenter
@ 2015-11-19 14:47   ` Christian Borntraeger
  -1 siblings, 0 replies; 6+ messages in thread
From: Christian Borntraeger @ 2015-11-19 14:47 UTC (permalink / raw)
  To: Dan Carpenter, Gleb Natapov, Janosch Frank
  Cc: Paolo Bonzini, kvm, kernel-janitors

On 11/19/2015 01:40 PM, Dan Carpenter wrote:
> The "goto out_err" is buggy because we forgot to set the return code.
> 
> The other issue is that if the kmalloc() fails, we should remove the
> debugfs directory before returning.
> 
> Fixes: 7805f53a85ec ('KVM: Create debugfs dir and stat files for each VM')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Dan

this patch is not yet part of the kvm repo. It was posted for review
and I added it on the kvms390 linux next branch. So I assume you have gotten
that patch via next?


Given that there might be more potential feedback and
given that I will respin my next branch (I have no consumers, so my next
branch is rebased from time to time), and then the patch will go
from my tree via Paolos kvm tree into Linus tree, I would prefer to fold in
your patches and adopt the commit message to give you some credit.
ok with you?

Christian
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 1ee2f73..f62621f 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -571,12 +571,14 @@ static int kvm_create_vm_debugfs(struct kvm *kvm)
>  	snprintf(dir_name, sizeof(dir_name), "%d", current->pid);
>  	kvm->debugfs_dentry = debugfs_create_dir(dir_name, kvm_debugfs_dir);
>  	if (!kvm->debugfs_dentry)
> -		goto out_err;
> +		return -ENOMEM;
> 
>  	kvm->debugfs_data = kmalloc(sizeof(*kvm->debugfs_data) *
>  				    kvm_debugfs_num_entries, GFP_KERNEL);
> -	if (!kvm->debugfs_data)
> -		return -ENOMEM;
> +	if (!kvm->debugfs_data) {
> +		r = -ENOMEM;
> +		goto err_remove_dir;
> +	}
> 
>  	for (p = debugfs_entries; p->name; p++, i++) {
>  		stat_data = kzalloc(sizeof(*stat_data), GFP_KERNEL);
> @@ -600,13 +602,15 @@ static int kvm_create_vm_debugfs(struct kvm *kvm)
>  	return r;
> 
>  out_err_clean:
> -	debugfs_remove_recursive(kvm->debugfs_dentry);
>  	kfree(stat_data);
>  	for (i--; i >= 0; i--)
>  		kfree(kvm->debugfs_data[i]);
> 
>  	kfree(kvm->debugfs_data);
> -out_err:
> +
> +err_remove_dir:
> +	debugfs_remove_recursive(kvm->debugfs_dentry);
> +
>  	return r;
>  }
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [patch 1/2] KVM: fix error handling in kvm_create_vm_debugfs()
@ 2015-11-19 14:47   ` Christian Borntraeger
  0 siblings, 0 replies; 6+ messages in thread
From: Christian Borntraeger @ 2015-11-19 14:47 UTC (permalink / raw)
  To: Dan Carpenter, Gleb Natapov, Janosch Frank
  Cc: Paolo Bonzini, kvm, kernel-janitors

On 11/19/2015 01:40 PM, Dan Carpenter wrote:
> The "goto out_err" is buggy because we forgot to set the return code.
> 
> The other issue is that if the kmalloc() fails, we should remove the
> debugfs directory before returning.
> 
> Fixes: 7805f53a85ec ('KVM: Create debugfs dir and stat files for each VM')
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Dan

this patch is not yet part of the kvm repo. It was posted for review
and I added it on the kvms390 linux next branch. So I assume you have gotten
that patch via next?


Given that there might be more potential feedback and
given that I will respin my next branch (I have no consumers, so my next
branch is rebased from time to time), and then the patch will go
from my tree via Paolos kvm tree into Linus tree, I would prefer to fold in
your patches and adopt the commit message to give you some credit.
ok with you?

Christian
> 
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 1ee2f73..f62621f 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -571,12 +571,14 @@ static int kvm_create_vm_debugfs(struct kvm *kvm)
>  	snprintf(dir_name, sizeof(dir_name), "%d", current->pid);
>  	kvm->debugfs_dentry = debugfs_create_dir(dir_name, kvm_debugfs_dir);
>  	if (!kvm->debugfs_dentry)
> -		goto out_err;
> +		return -ENOMEM;
> 
>  	kvm->debugfs_data = kmalloc(sizeof(*kvm->debugfs_data) *
>  				    kvm_debugfs_num_entries, GFP_KERNEL);
> -	if (!kvm->debugfs_data)
> -		return -ENOMEM;
> +	if (!kvm->debugfs_data) {
> +		r = -ENOMEM;
> +		goto err_remove_dir;
> +	}
> 
>  	for (p = debugfs_entries; p->name; p++, i++) {
>  		stat_data = kzalloc(sizeof(*stat_data), GFP_KERNEL);
> @@ -600,13 +602,15 @@ static int kvm_create_vm_debugfs(struct kvm *kvm)
>  	return r;
> 
>  out_err_clean:
> -	debugfs_remove_recursive(kvm->debugfs_dentry);
>  	kfree(stat_data);
>  	for (i--; i >= 0; i--)
>  		kfree(kvm->debugfs_data[i]);
> 
>  	kfree(kvm->debugfs_data);
> -out_err:
> +
> +err_remove_dir:
> +	debugfs_remove_recursive(kvm->debugfs_dentry);
> +
>  	return r;
>  }
> 


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [patch 1/2] KVM: fix error handling in kvm_create_vm_debugfs()
  2015-11-19 14:47   ` Christian Borntraeger
@ 2015-11-19 19:07     ` Dan Carpenter
  -1 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2015-11-19 19:07 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: Gleb Natapov, Janosch Frank, Paolo Bonzini, kvm, kernel-janitors

No problem.  Fold away.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 6+ messages in thread
* Re: [patch 1/2] KVM: fix error handling in kvm_create_vm_debugfs()
@ 2015-11-19 19:07     ` Dan Carpenter
  0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2015-11-19 19:07 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: Gleb Natapov, Janosch Frank, Paolo Bonzini, kvm, kernel-janitors

No problem.  Fold away.

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-11-19 19:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-19 12:40 [patch 1/2] KVM: fix error handling in kvm_create_vm_debugfs() Dan Carpenter
2015-11-19 12:40 ` Dan Carpenter
2015-11-19 14:47 ` Christian Borntraeger
2015-11-19 14:47   ` Christian Borntraeger
2015-11-19 19:07   ` Dan Carpenter
2015-11-19 19:07     ` Dan Carpenter

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.