[PATCH] sanitize anon_inode_getfd()

a) none of the callers even looks at inode or file returned by anon_inode_getfd()
b) any caller that would try to look at those would be racy, since by the time
it returns we might have raced with close() from another thread and that
file would be pining for fjords.

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

diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index f42be069e085b2d212b8cc633d61f418be6801f5..977ef208c05193bbe555a4b4bce50853f0483616 100644 (file)
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -57,9 +57,6 @@ static struct dentry_operations anon_inodefs_dentry_operations = {
  *                    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
  * @priv     [in]    private data for the new file (will be file's private_data)
@@ -68,10 +65,9 @@ static struct dentry_operations anon_inodefs_dentry_operations = {
  * that do not need to have a full-fledged inode in order to operate correctly.
  * 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.
+ * setup.  Returns new descriptor or -error.
  */
-int anon_inode_getfd(int *pfd, struct inode **pinode, struct file **pfile,
-                    const char *name, const struct file_operations *fops,
+int anon_inode_getfd(const char *name, const struct file_operations *fops,
                     void *priv)
 {
        struct qstr this;
@@ -125,10 +121,7 @@ int anon_inode_getfd(int *pfd, struct inode **pinode, struct file **pfile,
 
        fd_install(fd, file);
 
-       *pfd = fd;
-       *pinode = anon_inode_inode;
-       *pfile = file;
-       return 0;
+       return fd;
 
 err_dput:
        dput(dentry);

diff --git a/fs/eventfd.c b/fs/eventfd.c
index a9f130cd50ac2be6009ed2091fce0ec21d134fdd..343942deeec138b1925e7815d0ac819e41f1214e 100644 (file)
--- a/fs/eventfd.c
+++ b/fs/eventfd.c
@@ -200,10 +200,8 @@ struct file *eventfd_fget(int fd)
 
 asmlinkage long sys_eventfd(unsigned int count)
 {
-       int error, fd;
+       int fd;
        struct eventfd_ctx *ctx;
-       struct file *file;
-       struct inode *inode;
 
        ctx = kmalloc(sizeof(*ctx), GFP_KERNEL);
        if (!ctx)
@@ -216,12 +214,9 @@ 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);
-       if (!error)
-               return fd;
-
-       kfree(ctx);
-       return error;
+       fd = anon_inode_getfd("[eventfd]", &eventfd_fops, ctx);
+       if (fd < 0)
+               kfree(ctx);
+       return fd;
 }
 

diff --git a/fs/eventpoll.c b/fs/eventpoll.c
index 221086fef1743545c143a270bd1bc228cfb33561..990c01d2d66bcce64a1c2a993cd6cc01ea5951d5 100644 (file)
--- a/fs/eventpoll.c
+++ b/fs/eventpoll.c
@@ -1050,8 +1050,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));
@@ -1061,29 +1059,24 @@ asmlinkage long sys_epoll_create(int size)
         * structure ( "struct eventpoll" ).
         */
        error = -EINVAL;
-       if (size <= 0 || (error = ep_alloc(&ep)) != 0)
+       if (size <= 0 || (error = ep_alloc(&ep)) < 0) {
+               fd = error;
                goto error_return;
+       }
 
        /*
         * 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);
-       if (error)
-               goto error_free;
+       fd = anon_inode_getfd("[eventpoll]", &eventpoll_fops, ep);
+       if (fd < 0)
+               ep_free(ep);
 
+error_return:
        DNPRINTK(3, (KERN_INFO "[%p] eventpoll: sys_epoll_create(%d) = %d\n",
                     current, size, fd));
 
        return fd;
-
-error_free:
-       ep_free(ep);
-error_return:
-       DNPRINTK(3, (KERN_INFO "[%p] eventpoll: sys_epoll_create(%d) = %d\n",
-                    current, size, error));
-       return error;
 }
 
 /*

diff --git a/fs/signalfd.c b/fs/signalfd.c
index 8ead0db359339ca7f5986e3b89ff3b245938557e..619725644c75e4b30d5ba379f9d40a391466efdb 100644 (file)
--- a/fs/signalfd.c
+++ b/fs/signalfd.c
@@ -207,11 +207,8 @@ static const struct file_operations signalfd_fops = {
 
 asmlinkage long sys_signalfd(int ufd, sigset_t __user *user_mask, size_t sizemask)
 {
-       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)))
@@ -230,12 +227,11 @@ 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]",
-                                        &signalfd_fops, ctx);
-               if (error)
-                       goto err_fdalloc;
+               ufd = anon_inode_getfd("[signalfd]", &signalfd_fops, ctx);
+               if (ufd < 0)
+                       kfree(ctx);
        } else {
-               file = fget(ufd);
+               struct file *file = fget(ufd);
                if (!file)
                        return -EBADF;
                ctx = file->private_data;
@@ -252,9 +248,4 @@ asmlinkage long sys_signalfd(int ufd, sigset_t __user *user_mask, size_t sizemas
        }
 
        return ufd;
-
-err_fdalloc:
-       kfree(ctx);
-       return error;
 }
-

diff --git a/fs/timerfd.c b/fs/timerfd.c
index 5400524e9cb1d2b2e40bf810a1282277e54eec98..d87d354ec42438a4a0794cf964999eb376053c73 100644 (file)
--- a/fs/timerfd.c
+++ b/fs/timerfd.c
@@ -181,10 +181,8 @@ static struct file *timerfd_fget(int fd)
 
 asmlinkage long sys_timerfd_create(int clockid, int flags)
 {
-       int error, ufd;
+       int ufd;
        struct timerfd_ctx *ctx;
-       struct file *file;
-       struct inode *inode;
 
        if (flags)
                return -EINVAL;
@@ -200,12 +198,9 @@ 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);
-       if (error) {
+       ufd = anon_inode_getfd("[timerfd]", &timerfd_fops, ctx);
+       if (ufd < 0)
                kfree(ctx);
-               return error;
-       }
 
        return ufd;
 }

diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h
index b2e1ba325b9a4c0d044ea20c6629bda8d4d27200..6129e58ca7c99407716acbf6482e2d02d09956e9 100644 (file)
--- a/include/linux/anon_inodes.h
+++ b/include/linux/anon_inodes.h
@@ -8,8 +8,7 @@
 #ifndef _LINUX_ANON_INODES_H
 #define _LINUX_ANON_INODES_H
 
-int anon_inode_getfd(int *pfd, struct inode **pinode, struct file **pfile,
-                    const char *name, const struct file_operations *fops,
+int anon_inode_getfd(const char *name, const struct file_operations *fops,
                     void *priv);
 
 #endif /* _LINUX_ANON_INODES_H */

diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index c82cf15730a1e85c751483f75944e8905c0baa10..e89338e2b04379d7c29e816de44e8b6824b7885e 100644 (file)
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -834,16 +834,9 @@ static const struct file_operations kvm_vcpu_fops = {
  */
 static int create_vcpu_fd(struct kvm_vcpu *vcpu)
 {
-       int fd, r;
-       struct inode *inode;
-       struct file *file;
-
-       r = anon_inode_getfd(&fd, &inode, &file,
-                            "kvm-vcpu", &kvm_vcpu_fops, vcpu);
-       if (r) {
+       int fd = anon_inode_getfd("kvm-vcpu", &kvm_vcpu_fops, vcpu);
+       if (fd < 0)
                kvm_put_kvm(vcpu->kvm);
-               return r;
-       }
        return fd;
 }
 
@@ -1168,19 +1161,15 @@ static const struct file_operations kvm_vm_fops = {
 
 static int kvm_dev_ioctl_create_vm(void)
 {
-       int fd, r;
-       struct inode *inode;
-       struct file *file;
+       int fd;
        struct kvm *kvm;
 
        kvm = kvm_create_vm();
        if (IS_ERR(kvm))
                return PTR_ERR(kvm);
-       r = anon_inode_getfd(&fd, &inode, &file, "kvm-vm", &kvm_vm_fops, kvm);
-       if (r) {
+       fd = anon_inode_getfd("kvm-vm", &kvm_vm_fops, kvm);
+       if (fd < 0)
                kvm_put_kvm(kvm);
-               return r;
-       }
 
        return fd;
 }