[Libguestfs] [nbdkit PATCH v2 06/17] cow, cache: Better mkostemp fallback

[Eric Blake]

In 2018, we added fallbacks for Haiku lacking mkostemp (one of the
atomic CLOEXEC interfaces proposed for future POSIX [1]); however,
that fallback has minor bugs: if mkstemp() fails, we blindly call
fcntl(-1) (which probably changes the errno later reported against
"mkostemp: %s: %m"); and although it is historically unlikely that any
other bits will be set, F_SETFD should be used in a read-modify-write
pattern rather than a blind overwrite pattern.

[1] http://austingroupbugs.net/view.php?id=411

Update these fallbacks to use our newly-added set_cloexec utility
function; neither place needs atomicity because they occur during
.load (where we are still more-or-less single-threaded), rather than
while another thread may be actively inside a plugin (such as sh)
using fork().

Fixes: 60076fbcfd
Fixes: b962272a56
Signed-off-by: Eric Blake <eblake at redhat.com>
---
 common/utils/utils.c |  5 ++++-
 filters/cache/blk.c  | 12 +++++++++++-
 filters/cow/blk.c    | 12 +++++++++++-
 3 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/common/utils/utils.c b/common/utils/utils.c
index 0548058c..7b63b4d4 100644
--- a/common/utils/utils.c
+++ b/common/utils/utils.c
@@ -140,12 +140,15 @@ exit_status_to_nbd_error (int status, const char *cmd)
  */
 int
 set_cloexec (int fd) {
-#ifdef SOCK_CLOEXEC
+#if defined SOCK_CLOEXEC && defined HAVE_MKOSTEMP
   nbdkit_error ("prefer creating fds with CLOEXEC atomically set");
   close (fd);
   errno = EBADF;
   return -1;
 #else
+# if defined SOCK_CLOEXEC || defined HAVE_MKOSTEMP
+# error "Unexpected: your system has incomplete atomic CLOEXEC support"
+# endif
   int f;
   int err;

diff --git a/filters/cache/blk.c b/filters/cache/blk.c
index cf7145d8..272d176e 100644
--- a/filters/cache/blk.c
+++ b/filters/cache/blk.c
@@ -109,8 +109,18 @@ blk_init (void)
 #ifdef HAVE_MKOSTEMP
   fd = mkostemp (template, O_CLOEXEC);
 #else
+  /* Not atomic, but this is only invoked during .load, so the race
+   * won't affect any plugin actions trying to fork
+   */
   fd = mkstemp (template);
-  fcntl (fd, F_SETFD, FD_CLOEXEC);
+  if (fd >= 0) {
+    fd = set_cloexec (fd);
+    if (fd < 0) {
+      int e = errno;
+      unlink (template);
+      errno = e;
+    }
+  }
 #endif
   if (fd == -1) {
     nbdkit_error ("mkostemp: %s: %m", tmpdir);
diff --git a/filters/cow/blk.c b/filters/cow/blk.c
index be43f2ff..2cae1122 100644
--- a/filters/cow/blk.c
+++ b/filters/cow/blk.c
@@ -114,8 +114,18 @@ blk_init (void)
 #ifdef HAVE_MKOSTEMP
   fd = mkostemp (template, O_CLOEXEC);
 #else
+  /* Not atomic, but this is only invoked during .load, so the race
+   * won't affect any plugin actions trying to fork
+   */
   fd = mkstemp (template);
-  fcntl (fd, F_SETFD, FD_CLOEXEC);
+  if (fd >= 0) {
+    fd = set_cloexec (fd);
+    if (fd < 0) {
+      int e = errno;
+      unlink (template);
+      errno = e;
+    }
+  }
 #endif
   if (fd == -1) {
     nbdkit_error ("mkostemp: %s: %m", tmpdir);
-- 
2.20.1