[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 3/4] Add a CLOEXEC recvfd
From: |
Bruno Haible |
Subject: |
Re: [PATCH 3/4] Add a CLOEXEC recvfd |
Date: |
Sun, 13 Mar 2011 17:16:30 +0100 |
User-agent: |
KMail/1.9.9 |
Hi Bastien,
> In order to avoid a race add a recvfd(int fd, int flags). flags could be
> O_CLOEXEC.
> ---
> lib/passfd.c | 58
> +++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> lib/passfd.h | 1 +
> m4/afunix.m4 | 22 +++++++++++++++++++++
> modules/passfd | 1 +
This is pretty good as well. But there's no need for two functions recvfd and
recvfd2. Better merge them into a single function. It's easy for the callers
to pass a 0 flags argument. In the POSIX API, a separate function was
introduced only when the previous function already existed for a long time,
like pipe()/pipe2() and accept()/accept4().
Also, it is better to test 'flags & O_CLOEXEC' instead of 'flags == O_CLOEXEC'
because
- The flags argument is conceptually a bit mask.
- O_CLOEXEC is 0 on some platforms (that don't support this flag), and
passing a 0 flags argument ought to _not_ set the close-on-exec flag.
Other than this, I applied the usual untabification and .m4 file indentation.
A possible simplification would be to assume that MSG_CMSG_CLOEXEC is a macro
when it exists, and thus simplify "#if HAVE_MSG_CMSG_CLOEXEC" to
"#if defined MSG_CMSG_CLOEXEC".
2011-03-13 Bastien Roucariès <address@hidden>
Bruno Haible <address@hidden>
passfd module, part 3.
* lib/passfd.h (recvfd): Add a flags argument.
* lib/passfd.c: Include <fcntl.h>, cloexec.h.
(recvfd): Add a flags argument.
* m4/afunix.m4 (gl_SOCKET_AFUNIX): Test whether MSG_CMSG_CLOEXEC
exists.
* modules/passfd (Depends-on): Add cloexec.
Suggested by Eric Blake.
--- lib/passfd.c.orig Sun Mar 13 16:26:20 2011
+++ lib/passfd.c Sun Mar 13 15:53:28 2011
@@ -19,6 +19,7 @@
#include "passfd.h"
#include <errno.h>
+#include <fcntl.h>
#include <stddef.h>
#include <stdlib.h>
#include <string.h>
@@ -35,6 +36,8 @@
# include <winsock2.h>
#endif
+#include "cloexec.h"
+
/* sendfd sends the file descriptor fd along the socket
to a process calling recvfd on the other end.
@@ -84,11 +87,12 @@
}
/* recvfd receives a file descriptor through the socket.
+ The flags are a bitmask, possibly including O_CLOEXEC (defined in
<fcntl.h>).
Return 0 on success, or -1 with errno set in case of error.
*/
int
-recvfd (int sock)
+recvfd (int sock, int flags)
{
char recv = 0;
const int mone = -1;
@@ -96,6 +100,12 @@
struct iovec iov[1];
struct msghdr msg;
+ if ((flags & ~O_CLOEXEC) != 0)
+ {
+ errno = EINVAL;
+ return -1;
+ }
+
/* send at least one char */
iov[0].iov_base = &recv;
iov[0].iov_len = 1;
@@ -108,6 +118,11 @@
#if HAVE_UNIXSOCKET_SCM_RIGHTS_BSD44_WAY
struct cmsghdr *cmsg;
char buf[CMSG_SPACE (sizeof (fd))];
+# if HAVE_MSG_CMSG_CLOEXEC
+ int flags_recvmsg = (flags & O_CLOEXEC ? MSG_CMSG_CLOEXEC : 0);
+# else
+ int flags_recvmsg = 0;
+# endif
msg.msg_control = buf;
msg.msg_controllen = sizeof (buf);
@@ -119,7 +134,7 @@
memcpy (CMSG_DATA (cmsg), &mone, sizeof (mone));
msg.msg_controllen = cmsg->cmsg_len;
- if (recvmsg (sock, &msg, 0) < 0)
+ if (recvmsg (sock, &msg, flags_recvmsg) < 0)
return -1;
cmsg = CMSG_FIRSTHDR (&msg);
@@ -133,13 +148,43 @@
}
memcpy (&fd, CMSG_DATA (cmsg), sizeof (fd));
+
+# if !HAVE_MSG_CMSG_CLOEXEC
+ /* set close-on-exec flag */
+ if (flags & O_CLOEXEC)
+ {
+ if (set_cloexec_flag (fd, true) < 0)
+ {
+ int saved_errno = errno;
+ (void) close (fd);
+ errno = saved_errno;
+ return -1;
+ }
+ }
+# endif
+
return fd;
+
#elif HAVE_UNIXSOCKET_SCM_RIGHTS_BSD43_WAY
msg.msg_accrights = &fd;
msg.msg_accrightslen = sizeof (fd);
if (recvmsg (sock, &msg, 0) < 0)
return -1;
+
+ /* set close-on-exec flag */
+ if (flags & O_CLOEXEC)
+ {
+ if (set_cloexec_flag (fd, true) < 0)
+ {
+ int saved_errno = errno;
+ (void) close (fd);
+ errno = saved_errno;
+ return -1;
+ }
+ }
+
return fd;
+
#else
errno = ENOSYS;
return -1;
--- lib/passfd.h.orig Sun Mar 13 16:26:20 2011
+++ lib/passfd.h Sun Mar 13 15:43:03 2011
@@ -23,7 +23,7 @@
#endif
extern int sendfd (int sock, int fd);
-extern int recvfd (int sock);
+extern int recvfd (int sock, int flags);
#ifdef __cplusplus
}
--- m4/afunix.m4.orig Sun Mar 13 16:26:20 2011
+++ m4/afunix.m4 Sun Mar 13 15:58:06 2011
@@ -1,4 +1,4 @@
-# afunix.m4 serial 2
+# afunix.m4 serial 3
dnl Copyright (C) 2011 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
@@ -109,4 +109,31 @@
AC_DEFINE([HAVE_UNIXSOCKET_SCM_RIGHTS_BSD43_WAY], [1],
[Define to 1 if fd can be sent/received in the BSD4.3 way.])
fi
+
+ AC_MSG_CHECKING([for UNIX domain sockets recvmsg() MSG_CMSG_CLOEXEC flag])
+ AC_CACHE_VAL([gl_cv_socket_unix_msg_cmsg_cloexec],
+ [AC_COMPILE_IFELSE(
+ [AC_LANG_PROGRAM(
+ [[#include <sys/types.h>
+ #ifdef HAVE_SYS_SOCKET_H
+ #include <sys/socket.h>
+ #endif
+ #ifdef HAVE_SYS_UN_H
+ #include <sys/un.h>
+ #endif
+ #ifdef HAVE_WINSOCK2_H
+ #include <winsock2.h>
+ #endif
+ ]],
+ [[int flags = MSG_CMSG_CLOEXEC;
+ if (&flags) return 0;
+ ]])],
+ [gl_cv_socket_unix_msg_cmsg_cloexec=yes],
+ [gl_cv_socket_unix_msg_cmsg_cloexec=no])
+ ])
+ AC_MSG_RESULT([$gl_cv_socket_unix_msg_cmsg_cloexec])
+ if test $gl_cv_socket_unix_msg_cmsg_cloexec = yes; then
+ AC_DEFINE([HAVE_MSG_CMSG_CLOEXEC], [1],
+ [Define to 1 if recvmsg could be specified with MSG_CMSG_CLOEXEC.])
+ fi
])
--- modules/passfd.orig Sun Mar 13 16:26:20 2011
+++ modules/passfd Sun Mar 13 16:03:40 2011
@@ -8,6 +8,7 @@
m4/sockpfaf.m4
Depends-on:
+cloexec
sys_socket
extensions
--
In memoriam Odette Sansom <http://en.wikipedia.org/wiki/Odette_Hallowes>
- [V4] Passfd, Bastien ROUCARIES, 2011/03/07
- [PATCH 2/4] sendfd, recvf pass file descriptors along Unix domain sockets, Bastien ROUCARIES, 2011/03/07
- [PATCH 4/4] Add test for passfd, Bastien ROUCARIES, 2011/03/07
- passfd on glibc, Bruno Haible, 2011/03/13
- passfd on OpenBSD, Bruno Haible, 2011/03/13
- passfd on FreeBSD, Bruno Haible, 2011/03/13
- passfd on AIX 5.2, Bruno Haible, 2011/03/13