kernel-janitors.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [patch] KVM: use after free in kvm_ioctl_create_device()
@ 2016-11-30 19:21 Dan Carpenter
  2016-12-01 12:20 ` David Hildenbrand
  2016-12-01 15:14 ` Radim Krčmář
  0 siblings, 2 replies; 3+ messages in thread
From: Dan Carpenter @ 2016-11-30 19:21 UTC (permalink / raw)
  To: Paolo Bonzini, Christoffer Dall
  Cc: Radim Krčmář, kvm, kernel-janitors

We should move the ops->destroy(dev) after the list_del(&dev->vm_node)
so that we don't use "dev" after freeing it.

Fixes: a28ebea2adc4 ("KVM: Protect device ops->create and list_add with kvm->lock")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 69a1247..de102ca 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -2899,10 +2899,10 @@ static int kvm_ioctl_create_device(struct kvm *kvm,
 
 	ret = anon_inode_getfd(ops->name, &kvm_device_fops, dev, O_RDWR | O_CLOEXEC);
 	if (ret < 0) {
-		ops->destroy(dev);
 		mutex_lock(&kvm->lock);
 		list_del(&dev->vm_node);
 		mutex_unlock(&kvm->lock);
+		ops->destroy(dev);
 		return ret;
 	}
 

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

* Re: [patch] KVM: use after free in kvm_ioctl_create_device()
  2016-11-30 19:21 [patch] KVM: use after free in kvm_ioctl_create_device() Dan Carpenter
@ 2016-12-01 12:20 ` David Hildenbrand
  2016-12-01 15:14 ` Radim Krčmář
  1 sibling, 0 replies; 3+ messages in thread
From: David Hildenbrand @ 2016-12-01 12:20 UTC (permalink / raw)
  To: Dan Carpenter, Paolo Bonzini, Christoffer Dall
  Cc: Radim Krčmář, kvm, kernel-janitors

Am 30.11.2016 um 20:21 schrieb Dan Carpenter:
> We should move the ops->destroy(dev) after the list_del(&dev->vm_node)
> so that we don't use "dev" after freeing it.
>
> Fixes: a28ebea2adc4 ("KVM: Protect device ops->create and list_add with kvm->lock")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> index 69a1247..de102ca 100644
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -2899,10 +2899,10 @@ static int kvm_ioctl_create_device(struct kvm *kvm,

Looks sane to me! This also matches kvm_destroy_devices().

Reviewed-by: David Hildenbrand <david@redhat.com>


-- 

David

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

* Re: [patch] KVM: use after free in kvm_ioctl_create_device()
  2016-11-30 19:21 [patch] KVM: use after free in kvm_ioctl_create_device() Dan Carpenter
  2016-12-01 12:20 ` David Hildenbrand
@ 2016-12-01 15:14 ` Radim Krčmář
  1 sibling, 0 replies; 3+ messages in thread
From: Radim Krčmář @ 2016-12-01 15:14 UTC (permalink / raw)
  To: Dan Carpenter; +Cc: Paolo Bonzini, Christoffer Dall, kvm, kernel-janitors

2016-11-30 22:21+0300, Dan Carpenter:
> We should move the ops->destroy(dev) after the list_del(&dev->vm_node)
> so that we don't use "dev" after freeing it.
> 
> Fixes: a28ebea2adc4 ("KVM: Protect device ops->create and list_add with kvm->lock")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

Applied to kvm/master, thanks.

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

end of thread, other threads:[~2016-12-01 15:14 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-11-30 19:21 [patch] KVM: use after free in kvm_ioctl_create_device() Dan Carpenter
2016-12-01 12:20 ` David Hildenbrand
2016-12-01 15:14 ` Radim Krčmář

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).