[Libguestfs] [nbdkit PATCH v2 12/17] filters: Set CLOEXEC on files opened during .config

[Eric Blake]

Any file descriptor left opened after .config must eventually be
marked CLOEXEC; otherwise, if our plugin fork()s (such as the sh
plugin), we leak an fd into that plugin (observed with the example in
the previous patch).

If atomicity matters, the options are fopen("we") (sadly, Haiku lacks
this), or open(O_CLOEXEC)/fdopen("w").  If atomicity does not matter,
you can also get by with fopen("w")/fcntl(F_SETFD).  But since Haiku
at least has O_CLOEXEC, and that version is less verbose, we don't
have to worry about the longer fcntl variant.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 filters/log/log.c     | 22 ++++++++++++++++++++--
 filters/stats/stats.c | 21 +++++++++++++++++++--
 2 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/filters/log/log.c b/filters/log/log.c
index 9c0f35cd..7cf741e1 100644
--- a/filters/log/log.c
+++ b/filters/log/log.c
@@ -42,6 +42,8 @@
 #include <pthread.h>
 #include <sys/time.h>
 #include <assert.h>
+#include <fcntl.h>
+#include <unistd.h>

 #include <nbdkit-filter.h>

@@ -88,13 +90,29 @@ log_config (nbdkit_next_config *next, void *nxdata,
 static int
 log_config_complete (nbdkit_next_config_complete *next, void *nxdata)
 {
+  int fd;
+
   if (!logfilename) {
     nbdkit_error ("missing logfile= parameter for the log filter");
     return -1;
   }
-  logfile = fopen (logfilename, append ? "a" : "w");
+  /* Using fopen("ae"/"we") would be more convenient, but as Haiku
+   * still lacks that, use this instead. Atomicity is not essential
+   * here since .config completes before threads that might fork, if
+   * we have to later add yet another fallback to fcntl(fileno()) for
+   * systems without O_CLOEXEC.
+   */
+  fd = open (logfilename,
+             O_CLOEXEC | O_WRONLY | O_CREAT | (append ? O_APPEND : O_TRUNC),
+             0666);
+  if (fd < 0) {
+    nbdkit_error ("open: %s: %m", logfilename);
+    return -1;
+  }
+  logfile = fdopen (fd, append ? "a" : "w");
   if (!logfile) {
-    nbdkit_error ("fopen: %m");
+    nbdkit_error ("fdopen: %s: %m", logfilename);
+    close (fd);
     return -1;
   }

diff --git a/filters/stats/stats.c b/filters/stats/stats.c
index 9631cb39..d8ff02bc 100644
--- a/filters/stats/stats.c
+++ b/filters/stats/stats.c
@@ -39,6 +39,8 @@
 #include <inttypes.h>
 #include <string.h>
 #include <sys/time.h>
+#include <fcntl.h>
+#include <unistd.h>

 #include <pthread.h>

@@ -139,14 +141,29 @@ stats_config (nbdkit_next_config *next, void *nxdata,
 static int
 stats_config_complete (nbdkit_next_config_complete *next, void *nxdata)
 {
+  int fd;
+
   if (filename == NULL) {
     nbdkit_error ("stats filter requires statsfile parameter");
     return -1;
   }

-  fp = fopen (filename, append ? "a" : "w");
+  /* Using fopen("ae"/"we") would be more convenient, but as Haiku
+   * still lacks that, use this instead. Atomicity is not essential
+   * here since .config completes before threads that might fork, if
+   * we have to later add yet another fallback to fcntl(fileno()) for
+   * systems without O_CLOEXEC.
+   */
+  fd = open (filename,
+             O_CLOEXEC | O_WRONLY | O_CREAT | (append ? O_APPEND : O_TRUNC),
+             0666);
+  if (fd < 0) {
+    nbdkit_error ("open: %s: %m", filename);
+    return -1;
+  }
+  fp = fdopen (fd, append ? "a" : "w");
   if (fp == NULL) {
-    nbdkit_error ("%s: %m", filename);
+    nbdkit_error ("fdopen: %s: %m", filename);
     return -1;
   }

-- 
2.20.1