[Libguestfs] [nbdkit PATCH 7/8] filters: Set CLOEXEC on files opened during .config

Eric Blake eblake at redhat.com
Wed Jul 31 21:31:35 UTC 2019


Any file descriptor opened during .config must eventually be marked
CLOEXEC; otherwise, if our plugin fork()s (such as the sh plugin), we
leak an fd into that plugin (easy enough to observe by modifying the
example in the previous patch to add --filter=log or --filter=stats).
However, we need not worry about atomic CLOEXEC, as config is
effectively single-threaded and before any plugin code in another
thread will be attempting a fork(); put another way, we don't have to
worry about whether fopen("we") is univerally supported yet.

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

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

 #include <nbdkit-filter.h>

@@ -88,13 +89,30 @@ log_config (nbdkit_next_config *next, void *nxdata,
 static int
 log_config_complete (nbdkit_next_config_complete *next, void *nxdata)
 {
+  int f;
+
   if (!logfilename) {
     nbdkit_error ("missing logfile= parameter for the log filter");
     return -1;
   }
+  /* Using fopen("ae"/"we") would be more convenient, but as .config
+   * is called before other threads can be executing plugin code,
+   * using the older racy paradigm for portability doesn't hurt.
+   */
   logfile = fopen (logfilename, append ? "a" : "w");
   if (!logfile) {
-    nbdkit_error ("fopen: %m");
+    nbdkit_error ("fopen: %s: %m", logfilename);
+    return -1;
+  }
+  f = fcntl (fileno (logfile), F_GETFD);
+  if (f == -1) {
+    nbdkit_error ("fcntl: %s: %m", logfilename);
+    fclose (logfile);
+    return -1;
+  }
+  if (fcntl (fileno (logfile), F_SETFD, f | FD_CLOEXEC) == -1) {
+    nbdkit_error ("fcntl: %s: %m", logfilename);
+    fclose (logfile);
     return -1;
   }

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

 #include <pthread.h>

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

+  /* Using fopen("ae"/"we") would be more convenient, but as .config
+   * is called before other threads can be executing plugin code,
+   * using the older racy paradigm for portability doesn't hurt.
+   */
   fp = fopen (filename, append ? "a" : "w");
   if (fp == NULL) {
-    nbdkit_error ("%s: %m", filename);
+    nbdkit_error ("fopen: %s: %m", filename);
     return -1;
   }
+  f = fcntl (fileno (fp), F_GETFD);
+  if (f == -1) {
+    nbdkit_error ("fcntl: %s: %m", filename);
+    fclose (fp);
+  }
+  if (fcntl (fileno (fp), F_SETFD, f | FD_CLOEXEC) == -1) {
+    nbdkit_error ("fcntl: %s: %m", filename);
+    fclose (fp);
+  }

   gettimeofday (&start_t, NULL);

-- 
2.20.1




More information about the Libguestfs mailing list