[Devel] [PATCH RHEL7 COMMIT] ms/net: handle error more gracefully in socketpair()

[Vasily Averin]

The commit is pushed to "branch-rh7-3.10.0-1160.6.1.vz7.171.x-ovz" and will appear at https://src.openvz.org/scm/ovz/vzkernel.git
after rh7-3.10.0-1160.6.1.vz7.171.1
------>
commit a90919887055cb48048c71d24102324addcb5af8
Author: Yann Droneaud <ydroneaud at opteya.com>
Date:   Thu Dec 3 12:31:13 2020 +0300

    ms/net: handle error more gracefully in socketpair()
    
    This patch makes socketpair() use error paths which do not
    rely on heavy-weight call to sys_close(): it's better to try
    to push the file descriptor to userspace before installing
    the socket file to the file descriptor, so that errors are
    catched earlier and being easier to handle.
    
    Using sys_close() seems to be the exception, while writing the
    file descriptor before installing it look like it's more or less
    the norm: eg. except for code used in init/, error handling
    involve fput() and put_unused_fd(), but not sys_close().
    
    This make socketpair() usage of sys_close() quite unusual.
    So it deserves to be replaced by the common pattern relying on
    fput() and put_unused_fd() just like, for example, the one used
    in pipe(2) or recvmsg(2).
    
    Three distinct error paths are still needed since calling
    fput() on file structure returned by sock_alloc_file() will
    implicitly call sock_release() on the associated socket
    structure.
    
    Cc: David S. Miller <davem at davemloft.net>
    Cc: Al Viro <viro at zeniv.linux.org.uk>
    Signed-off-by: Yann Droneaud <ydroneaud at opteya.com>
    Link: http://marc.info/?i=1385979146-13825-1-git-send-email-ydroneaud@opteya.com
    Signed-off-by: David S. Miller <davem at davemloft.net>
    
    (cherry-picked from commit d73aa2867f33582314f098277421ded65f5745a9)
    https://syzkaller.appspot.com/bug?id=23d0799f98b7c8c4bc3f5ecff7c28a87c64e40e8
    https://jira.sw.ru/browse/PSBM-123046
    Signed-off-by: Vasily Averin <vvs at virtuozzo.com>
---
 net/socket.c | 49 +++++++++++++++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 18 deletions(-)

diff --git a/net/socket.c b/net/socket.c
index 22acff9..dd8fa2d 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1533,48 +1533,61 @@ SYSCALL_DEFINE4(socketpair, int, family, int, type, int, protocol,
 		err = fd1;
 		goto out_release_both;
 	}
+
 	fd2 = get_unused_fd_flags(flags);
 	if (unlikely(fd2 < 0)) {
 		err = fd2;
-		put_unused_fd(fd1);
-		goto out_release_both;
+		goto out_put_unused_1;
 	}
 
 	newfile1 = sock_alloc_file(sock1, flags, NULL);
 	if (IS_ERR(newfile1)) {
 		err = PTR_ERR(newfile1);
-		put_unused_fd(fd1);
-		put_unused_fd(fd2);
-		goto out_release_both;
+		goto out_put_unused_both;
 	}
 
 	newfile2 = sock_alloc_file(sock2, flags, NULL);
 	if (IS_ERR(newfile2)) {
 		err = PTR_ERR(newfile2);
-		fput(newfile1);
-		put_unused_fd(fd1);
-		put_unused_fd(fd2);
-		sock_release(sock2);
-		goto out;
+		goto out_fput_1;
 	}
 
+	err = put_user(fd1, &usockvec[0]);
+	if (err)
+		goto out_fput_both;
+
+	err = put_user(fd2, &usockvec[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.
 	 */
 
-	err = put_user(fd1, &usockvec[0]);
-	if (!err)
-		err = put_user(fd2, &usockvec[1]);
-	if (!err)
-		return 0;
+	return 0;
 
-	sys_close(fd2);
-	sys_close(fd1);
-	return err;
+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:
+	put_unused_fd(fd2);
+out_put_unused_1:
+	put_unused_fd(fd1);
 out_release_both:
 	sock_release(sock2);
 out_release_1: