[Libguestfs] [nbdkit PATCH 5/8] cow, cache: Better mkostemp fallback

[Eric Blake]

Haiku is seriously behind in adding support for atomic CLOEXEC; in
2018, we added fallback support since it does not yet provide mkostemp
as requested by an upcoming POSIX addition [1].  However, the fallback
we added is rather brain-dead - 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.

The fact that our fallback is non-atomic is not a problem in practice,
since we are only using this code 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().

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

Fixes: 60076fbcfd
Fixes: b962272a56
Signed-off-by: Eric Blake <eblake at redhat.com>
---
 filters/cache/blk.c | 19 ++++++++++++++++++-
 filters/cow/blk.c   | 19 ++++++++++++++++++-
 2 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/filters/cache/blk.c b/filters/cache/blk.c
index cf7145d8..1fa9ea43 100644
--- a/filters/cache/blk.c
+++ b/filters/cache/blk.c
@@ -109,8 +109,25 @@ 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) {
+    int f = fcntl (fd, F_GETFD);
+    if (f == -1) {
+      nbdkit_error ("fcntl F_GETFD: %s: %m", template);
+      close (fd);
+      unlink (template);
+      return -1;
+    }
+    if (fcntl (fd, F_SETFD, f | FD_CLOEXEC) == -1) {
+      nbdkit_error ("fcntl F_SETFD: %s: %m", template);
+      close (fd);
+      unlink (template);
+      return -1;
+    }
+  }
 #endif
   if (fd == -1) {
     nbdkit_error ("mkostemp: %s: %m", tmpdir);
diff --git a/filters/cow/blk.c b/filters/cow/blk.c
index be43f2ff..12ad7820 100644
--- a/filters/cow/blk.c
+++ b/filters/cow/blk.c
@@ -114,8 +114,25 @@ 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) {
+    int f = fcntl (fd, F_GETFD);
+    if (f == -1) {
+      nbdkit_error ("fcntl F_GETFD: %s: %m", template);
+      close (fd);
+      unlink (template);
+      return -1;
+    }
+    if (fcntl (fd, F_SETFD, f | FD_CLOEXEC) == -1) {
+      nbdkit_error ("fcntl F_SETFD: %s: %m", template);
+      close (fd);
+      unlink (template);
+      return -1;
+    }
+  }
 #endif
   if (fd == -1) {
     nbdkit_error ("mkostemp: %s: %m", tmpdir);
-- 
2.20.1