[PATCH] fix race in kvm_dev_ioctl_create_vm(), sanitize anon_inode_getfd()

a) none of the callers even looks at inode returned by anon_inode_getfd()
b) none of correct callers looks at file returned by anon_inode_getfd()
c) the only caller that does is (inevitably) racy, since by the time
it returns we might have raced with close() from another thread and that
file would be pining for fjords.

Fixed by adding a variant that pins the file (used by the only odd caller)
and by turning anon_inode_getfd() into wrapper around it.  And losing the
damn pfile and pinode from anon_inode_getfd(), to prevent similar breakage
in the future.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

5 files changed, 26 insertions, 21 deletions

diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index f42be069e085..fbcb87ab1054 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -53,12 +53,11 @@ static struct dentry_operations anon_inodefs_dentry_operations = {
 };
 
 /**
- * anon_inode_getfd - creates a new file instance by hooking it up to an
+ * __anon_inode_getfd - creates a new file instance by hooking it up to an
  *                    anonymous inode, and a dentry that describe the "class"
  *                    of the file
  *
  * @pfd:     [out]   pointer to the file descriptor
- * @dpinode: [out]   pointer to the inode
  * @pfile:   [out]   pointer to the file struct
  * @name:    [in]    name of the "class" of the new file
  * @fops     [in]    file operations for the new file
@@ -69,8 +68,13 @@ static struct dentry_operations anon_inodefs_dentry_operations = {
  * All the files created with anon_inode_getfd() will share a single inode,
  * hence saving memory and avoiding code duplication for the file/inode/dentry
  * setup.
+ * File will be pinned down; the caller should fput() it once it's done looking
+ * at the damn thing (or just use anon_inode_getfd()).  That reference to file
+ * is in addition to one created by inserting it into descriptor table.  Note
+ * that *pfd might be closed by the time we return - another thread might have
+ * guessed it and called close(2).
  */
-int anon_inode_getfd(int *pfd, struct inode **pinode, struct file **pfile,
+int __anon_inode_getfd(int *pfd, struct file **pfile,
 		     const char *name, const struct file_operations *fops,
 		     void *priv)
 {
@@ -123,10 +127,10 @@ int anon_inode_getfd(int *pfd, struct inode **pinode, struct file **pfile,
 	file->f_version = 0;
 	file->private_data = priv;
 
+	get_file(file);
 	fd_install(fd, file);
 
 	*pfd = fd;
-	*pinode = anon_inode_inode;
 	*pfile = file;
 	return 0;
 
@@ -136,6 +140,18 @@ err_put_unused_fd:
 	put_unused_fd(fd);
 	return error;
 }
+EXPORT_SYMBOL_GPL(__anon_inode_getfd);
+
+int anon_inode_getfd(int *pfd, const char *name,
+		     const struct file_operations *fops,
+		     void *priv)
+{
+	struct file *file;
+	int error = __anon_inode_getfd(pfd, &file, name, fops, priv);
+	if (!error)
+		fput(file);
+	return error;
+}
 EXPORT_SYMBOL_GPL(anon_inode_getfd);
 
 /*
diff --git a/fs/eventfd.c b/fs/eventfd.c
index a9f130cd50ac..07eef3ea46e7 100644
--- a/fs/eventfd.c
+++ b/fs/eventfd.c
@@ -202,8 +202,6 @@ asmlinkage long sys_eventfd(unsigned int count)
 {
 	int error, fd;
 	struct eventfd_ctx *ctx;
-	struct file *file;
-	struct inode *inode;
 
 	ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
 	if (!ctx)
@@ -216,8 +214,7 @@ asmlinkage long sys_eventfd(unsigned int count)
 	 * When we call this, the initialization must be complete, since
 	 * anon_inode_getfd() will install the fd.
 	 */
-	error = anon_inode_getfd(&fd, &inode, &file, "[eventfd]",
-				 &eventfd_fops, ctx);
+	error = anon_inode_getfd(&fd, "[eventfd]", &eventfd_fops, ctx);
 	if (!error)
 		return fd;
 
diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index a415f42d32cf..369b665efaaf 100644
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -1071,8 +1071,6 @@ asmlinkage long sys_epoll_create(int size)
 {
 	int error, fd = -1;
 	struct eventpoll *ep;
-	struct inode *inode;
-	struct file *file;
 
 	DNPRINTK(3, (KERN_INFO "[%p] eventpoll: sys_epoll_create(%d)\n",
 		     current, size));
@@ -1087,10 +1085,9 @@ asmlinkage long sys_epoll_create(int size)
 
 	/*
 	 * Creates all the items needed to setup an eventpoll file. That is,
-	 * a file structure, and inode and a free file descriptor.
+	 * a file structure and a free file descriptor.
 	 */
-	error = anon_inode_getfd(&fd, &inode, &file, "[eventpoll]",
-				 &eventpoll_fops, ep);
+	error = anon_inode_getfd(&fd, "[eventpoll]", &eventpoll_fops, ep);
 	if (error)
 		goto error_free;
 
diff --git a/fs/signalfd.c b/fs/signalfd.c
index cb2b63ae0bf4..a15592afb70d 100644
--- a/fs/signalfd.c
+++ b/fs/signalfd.c
@@ -205,8 +205,6 @@ asmlinkage long sys_signalfd(int ufd, sigset_t __user *user_mask, size_t sizemas
 	int error;
 	sigset_t sigmask;
 	struct signalfd_ctx *ctx;
-	struct file *file;
-	struct inode *inode;
 
 	if (sizemask != sizeof(sigset_t) ||
 	    copy_from_user(&sigmask, user_mask, sizeof(sigmask)))
@@ -225,12 +223,12 @@ asmlinkage long sys_signalfd(int ufd, sigset_t __user *user_mask, size_t sizemas
 		 * When we call this, the initialization must be complete, since
 		 * anon_inode_getfd() will install the fd.
 		 */
-		error = anon_inode_getfd(&ufd, &inode, &file, "[signalfd]",
+		error = anon_inode_getfd(&ufd, "[signalfd]",
 					 &signalfd_fops, ctx);
 		if (error)
 			goto err_fdalloc;
 	} else {
-		file = fget(ufd);
+		struct file *file = fget(ufd);
 		if (!file)
 			return -EBADF;
 		ctx = file->private_data;
diff --git a/fs/timerfd.c b/fs/timerfd.c
index 10c80b59ec4b..1ac728dc40bb 100644
--- a/fs/timerfd.c
+++ b/fs/timerfd.c
@@ -182,8 +182,6 @@ asmlinkage long sys_timerfd_create(int clockid, int flags)
 {
 	int error, ufd;
 	struct timerfd_ctx *ctx;
-	struct file *file;
-	struct inode *inode;
 
 	if (flags)
 		return -EINVAL;
@@ -199,8 +197,7 @@ asmlinkage long sys_timerfd_create(int clockid, int flags)
 	ctx->clockid = clockid;
 	hrtimer_init(&ctx->tmr, clockid, HRTIMER_MODE_ABS);
 
-	error = anon_inode_getfd(&ufd, &inode, &file, "[timerfd]",
-				 &timerfd_fops, ctx);
+	error = anon_inode_getfd(&ufd, "[timerfd]", &timerfd_fops, ctx);
 	if (error) {
 		kfree(ctx);
 		return error;