[PATCHv5 4/5] Allow setting O_NONBLOCK flag for new sockets

[PATCHv5 4/5] Allow setting O_NONBLOCK flag for new sockets

[Ulrich Drepper]

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(-)


--- linux/net/socket.c
+++ linux/net/socket.c
@@ -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;
 

Re: [PATCHv5 4/5] Allow setting O_NONBLOCK flag for new sockets

[Eric Dumazet]

Ulrich Drepper a écrit :
> This patch adds support for setting the O_NONBLOCK flag of the file
> descriptors returned by socket, socketpair, and accept.
> 

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

Re: [PATCHv5 4/5] Allow setting O_NONBLOCK flag for new sockets

[Ulrich Drepper]

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

Eric Dumazet wrote:
> 1) Can the fd passing with recvmsg() on AF_UNIX also gets O_CLOEXEC
> support ?

Already there, see MSG_CMSG_CLOEXEC.


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

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-----

Re: [PATCHv5 4/5] Allow setting O_NONBLOCK flag for new sockets

[Eric Dumazet]

[-- Attachment #1: Type: text/plain, Size: 883 bytes --]

Ulrich Drepper a écrit :
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> Eric Dumazet wrote:
>> 1) Can the fd passing with recvmsg() on AF_UNIX also gets O_CLOEXEC
>> support ?
> 
> Already there, see MSG_CMSG_CLOEXEC.

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
)

>> 2) Why this O_NONBLOCK ability is needed for sockets ? Is it a security
>> issue, and if yes could you remind it to me ?
> 
> 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.
> 

OK, thank you.


[-- Attachment #2: scm.patch --]
[-- Type: text/plain, Size: 531 bytes --]

diff --git a/net/core/scm.c b/net/core/scm.c
index 100ba6d..38a0b77 100644
--- a/net/core/scm.c
+++ b/net/core/scm.c
@@ -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;

Re: [PATCHv5 4/5] Allow setting O_NONBLOCK flag for new sockets

[Ulrich Drepper]

On Nov 24, 2007 12:28 AM, Eric Dumazet <dada1@cosmosbay.com> wrote:
> OK, but maybe for consistency, we might accept the two mechanisms.

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.