All of lore.kernel.org
 help / color / mirror / Atom feed
From: Avi Kivity <avi@redhat.com>
To: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: kvm@vger.kernel.org
Subject: Re: [PATCH/RFC] kvm: fix refcounting race release vs. module unload
Date: Sun, 23 Nov 2008 15:49:33 +0200	[thread overview]
Message-ID: <49295F6D.8010200@redhat.com> (raw)
In-Reply-To: <200811202010.00125.borntraeger@de.ibm.com>

Christian Borntraeger wrote:
> The problem is that kvm_destroy_vm can run while the module count
> is 0. That means, you can remove the module while kvm_destroy_vm
> is running. But kvm_destroy_vm is part of the module text. This
> causes a kerneloops. The race exists without the msleep but is much
> harder to trigger.
>
> Nevertheless, the right solution is to call kvm_destroy_vm only
> with module_ref_count > 0. This can be done by calling kvm_destroy_vm
> only via a release function, since the VFS will not allow module unload.
> This patch sets kvm_vcpu_fops.owner to the module and manually
> increases the module refcount after anon_inode_getfd, since
> anon_inode_getfd does not do it.
>
> Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
> ---
>  virt/kvm/kvm_main.c |    4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> Index: kvm/virt/kvm/kvm_main.c
> ===================================================================
> --- kvm.orig/virt/kvm/kvm_main.c
> +++ kvm/virt/kvm/kvm_main.c
> @@ -1303,7 +1303,7 @@ static int kvm_vcpu_release(struct inode
>  	return 0;
>  }
>  
> -static const struct file_operations kvm_vcpu_fops = {
> +static struct file_operations kvm_vcpu_fops = {
>  	.release        = kvm_vcpu_release,
>  	.unlocked_ioctl = kvm_vcpu_ioctl,
>  	.compat_ioctl   = kvm_vcpu_ioctl,
> @@ -1318,6 +1318,7 @@ static int create_vcpu_fd(struct kvm_vcp
>  	int fd = anon_inode_getfd("kvm-vcpu", &kvm_vcpu_fops, vcpu, 0);
>  	if (fd < 0)
>  		kvm_put_kvm(vcpu->kvm);
> +	__module_get(kvm_vcpu_fops.owner);
>  	return fd;
>  }
>  
> @@ -2061,6 +2062,7 @@ int kvm_init(void *opaque, unsigned int 
>  	}
>  
>  	kvm_chardev_ops.owner = module;
> +	kvm_vcpu_fops.owner = module;
>  
>  	r = misc_register(&kvm_dev);
>  	if (r) {
>   

Messing with module counts is slightly ugly. How about having a vm fd 
fget() the /dev/kvm fd() instead?



-- 
error compiling committee.c: too many arguments to function


  reply	other threads:[~2008-11-23 13:49 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-11-20 19:10 [PATCH/RFC] kvm: fix refcounting race release vs. module unload Christian Borntraeger
2008-11-23 13:49 ` Avi Kivity [this message]
2008-11-24  9:12   ` Christian Borntraeger
2008-11-25  8:07   ` [PATCH/RFC 2] kvm: fix module refcount issues with anon_inodegetfd Christian Borntraeger
2008-11-25 13:55     ` Avi Kivity
2008-11-27 14:01       ` [PATCH/Request for review]: check for fops->owner in anon_inode_getfd Christian Borntraeger
2008-11-27 18:49         ` Davide Libenzi
2008-11-27 19:17           ` [PATCH v2]: " Christian Borntraeger
2008-11-27 19:40             ` Davide Libenzi
2008-12-01  8:57               ` Christian Borntraeger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49295F6D.8010200@redhat.com \
    --to=avi@redhat.com \
    --cc=borntraeger@de.ibm.com \
    --cc=kvm@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.