2:26 p.m.
Haiku unfortunately lacks SOCK_CLOEXEC, so we have to plan for a
fallback at each site that uses it. In the testsuite, failure to use
CLOEXEC is harmless (in particular, this fixes a minor bug in
test-socket-activation where a failed socket followed by fcntl(-1)
corrupts errno). Meanwhile, in the server, we still need the fd to be
CLOEXEC, but at least don't have to worry about atomicity in the
current use.
In the process, note that since setting CLOEXEC often (but not always)
needs to be atomic, it is also worth tightening our set_cloexec helper
to refuse to run on capable platforms that should have used
SOCK_CLOEXEC; we'll add further restrictions as more atomic CLOEXEC
functions are audited.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
common/utils/utils.c | 12 ++++++++++++
server/sockets.c | 8 ++++++++
tests/test-socket-activation.c | 12 ++++++------
tests/web-server.c | 7 ++++++-
4 files changed, 32 insertions(+), 7 deletions(-)
diff --git a/common/utils/utils.c b/common/utils/utils.c
index f81a8433..0548058c 100644
--- a/common/utils/utils.c
+++ b/common/utils/utils.c
@@ -132,9 +132,20 @@ exit_status_to_nbd_error (int status, const char *cmd)
/* Set the FD_CLOEXEC flag on the given fd, if it is non-negative.
* On failure, close fd and return -1; on success, return fd.
+ *
+ * Note that this function should ONLY be used on platforms that lack
+ * atomic CLOEXEC support during fd creation (such as Haiku in 2019);
+ * when using it as a fallback path, you must also consider how to
+ * prevent fd leaks to plugins that want to fork().
*/
int
set_cloexec (int fd) {
+#ifdef SOCK_CLOEXEC
+ nbdkit_error ("prefer creating fds with CLOEXEC atomically set");
+ close (fd);
+ errno = EBADF;
+ return -1;
+#else
int f;
int err;
@@ -150,6 +161,7 @@ set_cloexec (int fd) {
return -1;
}
return fd;
+#endif
}
/* Set the O_NONBLOCK flag on the given fd, if it is non-negative.
diff --git a/server/sockets.c b/server/sockets.c
index b25405cb..5adf5af5 100644
--- a/server/sockets.c
+++ b/server/sockets.c
@@ -54,6 +54,7 @@
#include <pthread.h>
#include "internal.h"
+#include "utils.h"
static void
set_selinux_label (void)
@@ -107,7 +108,14 @@ bind_unix_socket (size_t *nr_socks)
set_selinux_label ();
+#ifdef SOCK_CLOEXEC
sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
+#else
+ /* Fortunately, this code is only run at startup, so there is no
+ * risk of the fd leaking to a plugin's fork()
+ */
+ sock = set_cloexec (socket (AF_UNIX, SOCK_STREAM, 0));
+#endif
if (sock == -1) {
perror ("bind_unix_socket: socket");
exit (EXIT_FAILURE);
diff --git a/tests/test-socket-activation.c b/tests/test-socket-activation.c
index f98a9d62..a7f9fa14 100644
--- a/tests/test-socket-activation.c
+++ b/tests/test-socket-activation.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2017 Red Hat Inc.
+ * Copyright (C) 2017-2019 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -58,6 +58,11 @@
#include <sys/un.h>
#include <sys/wait.h>
+#ifndef SOCK_CLOEXEC
+/* For this file, we don't care if fds are marked cloexec; leaking is okay. */
+#define SOCK_CLOEXEC 0
+#endif
+
#define FIRST_SOCKET_ACTIVATION_FD 3
#define NBDKIT_START_TIMEOUT 30 /* seconds */
@@ -215,12 +220,7 @@ main (int argc, char *argv[])
* not from the path), so we should be able to connect to the Unix
* domain socket by its path and receive an NBD magic string.
*/
-#ifdef SOCK_CLOEXEC
sock = socket (AF_UNIX, SOCK_STREAM|SOCK_CLOEXEC, 0);
-#else
- sock = socket (AF_UNIX, SOCK_STREAM, 0);
- fcntl (sock, F_SETFD, FD_CLOEXEC);
-#endif
if (sock == -1) {
perror ("socket");
exit (EXIT_FAILURE);
diff --git a/tests/web-server.c b/tests/web-server.c
index d79924ff..83f90881 100644
--- a/tests/web-server.c
+++ b/tests/web-server.c
@@ -1,5 +1,5 @@
/* nbdkit
- * Copyright (C) 2018 Red Hat Inc.
+ * Copyright (C) 2018-2019 Red Hat Inc.
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions are
@@ -50,6 +50,11 @@
#include <pthread.h>
+#ifndef SOCK_CLOEXEC
+/* For this file, we don't care if fds are marked cloexec; leaking is okay. */
+#define SOCK_CLOEXEC 0
+#endif
+
static char tmpdir[] = "/tmp/wsXXXXXX";
static char sockpath[] = "............./sock";
static int listen_sock = -1;
--
2.20.1