fix kcm_clone()

1) it's fput() or sock_release(), not both
2) don't do fd_install() until the last failure exit.
3) not a bug per se, but... don't attach socket to struct file
   until it's set up.

Take reserving descriptor into the caller, move fd_install() to the
caller, sanitize failure exits and calling conventions.

Cc: stable@vger.kernel.org # v4.6+
Acked-by: Tom Herbert <tom@herbertland.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: David S. Miller <davem@davemloft.net>

1 files changed, 27 insertions, 44 deletions

diff --git a/net/kcm/kcmsock.c b/net/kcm/kcmsock.c
index 0b750a22c4b9..c5fa634e63ca 100644
--- a/net/kcm/kcmsock.c
+++ b/net/kcm/kcmsock.c
@@ -1625,60 +1625,35 @@ static struct proto kcm_proto = {
 };
 
 /* Clone a kcm socket. */
-static int kcm_clone(struct socket *osock, struct kcm_clone *info,
-                     struct socket **newsockp)
+static struct file *kcm_clone(struct socket *osock)
 {
         struct socket *newsock;
         struct sock *newsk;
-        struct file *newfile;
-        int err, newfd;
+        struct file *file;
 
-        err = -ENFILE;
         newsock = sock_alloc();
         if (!newsock)
-                goto out;
+                return ERR_PTR(-ENFILE);
 
         newsock->type = osock->type;
         newsock->ops = osock->ops;
 
         __module_get(newsock->ops->owner);
 
-        newfd = get_unused_fd_flags(0);
-        if (unlikely(newfd < 0)) {
-                err = newfd;
-                goto out_fd_fail;
-        }
-
-        newfile = sock_alloc_file(newsock, 0, osock->sk->sk_prot_creator->name);
-        if (IS_ERR(newfile)) {
-                err = PTR_ERR(newfile);
-                goto out_sock_alloc_fail;
-        }
-
         newsk = sk_alloc(sock_net(osock->sk), PF_KCM, GFP_KERNEL,
                          &kcm_proto, true);
         if (!newsk) {
-                err = -ENOMEM;
-                goto out_sk_alloc_fail;
+                sock_release(newsock);
+                return ERR_PTR(-ENOMEM);
         }
-
         sock_init_data(newsock, newsk);
         init_kcm_sock(kcm_sk(newsk), kcm_sk(osock->sk)->mux);
 
-        fd_install(newfd, newfile);
-        *newsockp = newsock;
-        info->fd = newfd;
-
-        return 0;
+        file = sock_alloc_file(newsock, 0, osock->sk->sk_prot_creator->name);
+        if (IS_ERR(file))
+                sock_release(newsock);
 
-out_sk_alloc_fail:
-        fput(newfile);
-out_sock_alloc_fail:
-        put_unused_fd(newfd);
-out_fd_fail:
-        sock_release(newsock);
-out:
-        return err;
+        return file;
 }
 
 static int kcm_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
@@ -1708,17 +1683,25 @@ static int kcm_ioctl(struct socket *sock, unsigned int cmd, unsigned long arg)
         }
         case SIOCKCMCLONE: {
                 struct kcm_clone info;
-                struct socket *newsock = NULL;
-
-                err = kcm_clone(sock, &info, &newsock);
-                if (!err) {
-                        if (copy_to_user((void __user *)arg, &info,
-                                         sizeof(info))) {
-                                err = -EFAULT;
-                                sys_close(info.fd);
-                        }
-                }
+                struct file *file;
+
+                info.fd = get_unused_fd_flags(0);
+                if (unlikely(info.fd < 0))
+                        return info.fd;
 
+                file = kcm_clone(sock);
+                if (IS_ERR(file)) {
+                        put_unused_fd(info.fd);
+                        return PTR_ERR(file);
+                }
+                if (copy_to_user((void __user *)arg, &info,
+                                 sizeof(info))) {
+                        put_unused_fd(info.fd);
+                        fput(file);
+                        return -EFAULT;
+                }
+                fd_install(info.fd, file);
+                err = 0;
                 break;
         }
         default: