Allow setting O_NONBLOCK flag for new sockets

This patch adds support for setting the O_NONBLOCK flag of the file descriptors returned by socket, socketpair, and accept.

 socket.c |   15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

@@ -362,7 +362,7 @@ static int sock_alloc_fd(struct file **filep, int flags)
 	return fd;
 }
 
-static int sock_attach_fd(struct socket *sock, struct file *file)
+static int sock_attach_fd(struct socket *sock, struct file *file, int flags)
 {
 	struct dentry *dentry;
 	struct qstr name = { .name = "" };

@@ -384,7 +384,7 @@ static int sock_attach_fd(struct socket *sock, struct file *file)
 	init_file(file, sock_mnt, dentry, FMODE_READ | FMODE_WRITE,
 		  &socket_file_ops);
 	SOCK_INODE(sock)->i_fop = &socket_file_ops;
-	file->f_flags = O_RDWR;
+	file->f_flags = O_RDWR | (flags & O_NONBLOCK);
 	file->f_pos = 0;
 	file->private_data = sock;
 

@@ -397,7 +397,7 @@ static int sock_map_fd_flags(struct socket *sock, int flags)
 	int fd = sock_alloc_fd(&newfile, flags);
 
 	if (likely(fd >= 0)) {
-		int err = sock_attach_fd(sock, newfile);
+		int err = sock_attach_fd(sock, newfile, flags);
 
 		if (unlikely(err < 0)) {
 			put_filp(newfile);

@@ -1268,12 +1268,14 @@ asmlinkage long sys_socketpair(int family, int type, int protocol,
 		goto out_release_both;
 	}
 
-	err = sock_attach_fd(sock1, newfile1);
+	err = sock_attach_fd(sock1, newfile1,
+			     INDIRECT_PARAM(file_flags, flags));
 	if (unlikely(err < 0)) {
 		goto out_fd2;
 	}
 
-	err = sock_attach_fd(sock2, newfile2);
+	err = sock_attach_fd(sock2, newfile2,
+			     INDIRECT_PARAM(file_flags, flags));
 	if (unlikely(err < 0)) {
 		fput(newfile1);
 		goto out_fd1;

@@ -1423,7 +1425,8 @@ asmlinkage long sys_accept(int fd, struct sockaddr __user *upeer_sockaddr,
 		goto out_put;
 	}
 
-	err = sock_attach_fd(newsock, newfile);
+	err = sock_attach_fd(newsock, newfile,
+			     INDIRECT_PARAM(file_flags, flags));
 	if (err < 0)
 		goto out_fd_simple;
 

Thanks Ulrich for this v5 series. I have two more questions.

1) Can the fd passing with recvmsg() on AF_UNIX also gets O_CLOEXEC support ?

(In my understanding, only accept(), socket(), socketcall(), socketpair())
are handled, so it might work on i386 (because recvmsg() is multiplexed under socketcall), but not on x86_64.

2) Why this O_NONBLOCK ability is needed for sockets ? Is it a security issue, and if yes could you remind it to me ?

Thanks

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Already there, see MSG_CMSG_CLOEXEC.

No security issue. But look at any correct network program, all need to set the mode to non-blocking. Adding this support to the syscall comes at almost no cost and it cuts the cost for every program down by one or two syscalls.

- --
➧ Ulrich Drepper ➧ Red Hat, Inc. ➧ 444 Castro St ➧ Mountain View, CA ❖ -----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org

iD8DBQFHR9YQ2ijCOnn/RHQRArbyAJ0d25FPg/BWmJ4YIzJKhO9iaBJNXwCgmpuX PAA6u3Dc56AlBegTRqtqJPc=
=j5vi
-----END PGP SIGNATURE-----

OK, but maybe for consistency, we might accept the two mechanisms.

The one added in 2.6.23, and the more general indirect() way.

(See attached untested patch for the general idea, you'll have to also add #if INDSYSCALL(recvmsg)
case INDSYSCALL(recvmsg):
#endif
)

OK, thank you.

index 100ba6d..38a0b77 100644
@@ -230,11 +230,12 @@ void scm_detach_fds(struct msghdr *msg, struct scm_cookie *scm)
 	     i++, cmfptr++)
 	{
 		int new_fd;
+		int flags = INDIRECT_PARAM(file_flags, flags) & O_CLOEXEC;
 		err = security_file_receive(fp[i]);
 		if (err)
 			break;
 		err = get_unused_fd_flags(MSG_CMSG_CLOEXEC & msg->msg_flags
-					  ? O_CLOEXEC : 0);
+					  ? O_CLOEXEC : flags);
 		if (err < 0)
 			break;
 		new_fd = err;

It's not a question of the kernel interface. The issue with all these extensions is the userlevel interface. Ideally no new userlevel interface is needed. This is the case for open() and incidentally also for this case (through the flags parameter for recvmsg). For socket(), accept(), the situation is unfortunately different and we need a new interface.

With your proposed patch, we would have to introduce another recvmsg() interface to take advantage of the additional functionality. This just doesn't make any sense. This is no contest in aesthetics. You first have to think about the interface presented to the programmer at userlevel and then design the syscall interface. This is how MSG_CMSG_CLOEXEC came about.