From: Christian Borntraeger <borntraeger@de.ibm.com>
To: Davide Libenzi <davidel@xmailserver.org>
Cc: linux-fsdevel@vger.kernel.org, Avi Kivity <avi@redhat.com>,
kvm@vger.kernel.org
Subject: [PATCH v2]: check for fops->owner in anon_inode_getfd
Date: Thu, 27 Nov 2008 20:17:26 +0100 [thread overview]
Message-ID: <200811272017.26719.borntraeger@de.ibm.com> (raw)
In-Reply-To: <alpine.DEB.1.10.0811271034210.3147@alien.or.mcafeemobile.com>
> > + if (fops->owner && !try_module_get(fops->owner))
> > + return -ENOENT;
> > +
> > error = get_unused_fd_flags(flags);
> > if (error < 0)
> > return error;
>
> What if get_unused_fd_flags() (or the following error-returing ops) fails
> after a successful try_module_get()?
Right, well spotted. I have added a fixup label.
From: Christian Borntraeger <borntraeger@de.ibm.com>
There is an imbalance for anonymous inodes. If the fops->owner field is set,
the module reference count of owner is decreases on release.
("filp_close" --> "__fput" ---> "fops_put")
On the other hand, anon_inode_getfd does not increase the module reference
count of owner. This causes two problems:
- if owner is set, the module refcount goes negative
- if owner is not set, the module can be unloaded while code is running
This patch changes anon_inode_getfd to be symmetric regarding fops->owner
handling.
I have checked all existing users of anon_inode_getfd. Noone sets fops->owner,
thats why nobody has seen the module refcount negative.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
From: Christian Borntraeger <borntraeger@de.ibm.com>
There is an imbalance for anonymous inodes. If the fops->owner field is set,
the module reference count of owner is decreases on release.
("filp_close" --> "__fput" ---> "fops_put")
On the other hand, anon_inode_getfd does not increase the module reference
count of owner. This causes two problems:
- if owner is set, the module refcount goes negative
- if owner is not set, the module can be unloaded while code is running
This patch changes anon_inode_getfd to be symmetric regarding fops->owner
handling.
I have checked all existing users of anon_inode_getfd. Noone sets fops->owner,
thats why nobody has seen the module refcount negative.
Signed-off-by: Christian Borntraeger <borntraeger@de.ibm.com>
---
fs/anon_inodes.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
Index: kvm/fs/anon_inodes.c
===================================================================
--- kvm.orig/fs/anon_inodes.c
+++ kvm/fs/anon_inodes.c
@@ -79,9 +79,12 @@ int anon_inode_getfd(const char *name, c
if (IS_ERR(anon_inode_inode))
return -ENODEV;
+ if (fops->owner && !try_module_get(fops->owner))
+ return -ENOENT;
+
error = get_unused_fd_flags(flags);
if (error < 0)
- return error;
+ goto err_module;
fd = error;
/*
@@ -128,6 +131,8 @@ err_dput:
dput(dentry);
err_put_unused_fd:
put_unused_fd(fd);
+err_module:
+ module_put(fops->owner);
return error;
}
EXPORT_SYMBOL_GPL(anon_inode_getfd);
next prev parent reply other threads:[~2008-11-27 19:17 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
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 ` Christian Borntraeger [this message]
2008-11-27 19:40 ` [PATCH v2]: " 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=200811272017.26719.borntraeger@de.ibm.com \
--to=borntraeger@de.ibm.com \
--cc=avi@redhat.com \
--cc=davidel@xmailserver.org \
--cc=kvm@vger.kernel.org \
--cc=linux-fsdevel@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.