Re: [PATCH 2/3] socketpair(): allocate descriptors first

[David Miller]

From: Al Viro 
Date: Tue, 5 Dec 2017 23:28:38 +

> simplifies failure exits considerably...
> 
> Reviewed-by: Eric Dumazet 
> Signed-off-by: Al Viro 

Applied.

[PATCH 2/3] socketpair(): allocate descriptors first

[Al Viro]

simplifies failure exits considerably...

Reviewed-by: Eric Dumazet 
Signed-off-by: Al Viro 
---
 net/socket.c | 89 ++--
 1 file changed, 38 insertions(+), 51 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index 42d8e9c9ccd5..2df83c0bfde9 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1366,87 +1366,74 @@ SYSCALL_DEFINE4(socketpair, int, family, int, type, 
int, protocol,
flags = (flags & ~SOCK_NONBLOCK) | O_NONBLOCK;
 
/*
+* reserve descriptors and make sure we won't fail
+* to return them to userland.
+*/
+   fd1 = get_unused_fd_flags(flags);
+   if (unlikely(fd1 < 0))
+   return fd1;
+
+   fd2 = get_unused_fd_flags(flags);
+   if (unlikely(fd2 < 0)) {
+   put_unused_fd(fd1);
+   return fd2;
+   }
+
+   err = put_user(fd1, [0]);
+   if (err)
+   goto out;
+
+   err = put_user(fd2, [1]);
+   if (err)
+   goto out;
+
+   /*
 * Obtain the first socket and check if the underlying protocol
 * supports the socketpair call.
 */
 
err = sock_create(family, type, protocol, );
-   if (err < 0)
+   if (unlikely(err < 0))
goto out;
 
err = sock_create(family, type, protocol, );
-   if (err < 0)
-   goto out_release_1;
-
-   err = sock1->ops->socketpair(sock1, sock2);
-   if (err < 0)
-   goto out_release_both;
-
-   fd1 = get_unused_fd_flags(flags);
-   if (unlikely(fd1 < 0)) {
-   err = fd1;
-   goto out_release_both;
+   if (unlikely(err < 0)) {
+   sock_release(sock1);
+   goto out;
}
 
-   fd2 = get_unused_fd_flags(flags);
-   if (unlikely(fd2 < 0)) {
-   err = fd2;
-   goto out_put_unused_1;
+   err = sock1->ops->socketpair(sock1, sock2);
+   if (unlikely(err < 0)) {
+   sock_release(sock2);
+   sock_release(sock1);
+   goto out;
}
 
newfile1 = sock_alloc_file(sock1, flags, NULL);
if (IS_ERR(newfile1)) {
err = PTR_ERR(newfile1);
-   goto out_put_unused_both;
+   sock_release(sock1);
+   sock_release(sock2);
+   goto out;
}
 
newfile2 = sock_alloc_file(sock2, flags, NULL);
if (IS_ERR(newfile2)) {
err = PTR_ERR(newfile2);
-   goto out_fput_1;
+   sock_release(sock2);
+   fput(newfile1);
+   goto out;
}
 
-   err = put_user(fd1, [0]);
-   if (err)
-   goto out_fput_both;
-
-   err = put_user(fd2, [1]);
-   if (err)
-   goto out_fput_both;
-
audit_fd_pair(fd1, fd2);
 
fd_install(fd1, newfile1);
fd_install(fd2, newfile2);
-   /* fd1 and fd2 may be already another descriptors.
-* Not kernel problem.
-*/
-
return 0;
 
-out_fput_both:
-   fput(newfile2);
-   fput(newfile1);
-   put_unused_fd(fd2);
-   put_unused_fd(fd1);
-   goto out;
-
-out_fput_1:
-   fput(newfile1);
-   put_unused_fd(fd2);
-   put_unused_fd(fd1);
-   sock_release(sock2);
-   goto out;
-
-out_put_unused_both:
+out:
put_unused_fd(fd2);
-out_put_unused_1:
put_unused_fd(fd1);
-out_release_both:
-   sock_release(sock2);
-out_release_1:
-   sock_release(sock1);
-out:
return err;
 }
 
-- 
2.11.0