[Libguestfs] [nbdkit PATCH v2 08/17] rate: Atomically set CLOEXEC on fds

Eric Blake eblake at redhat.com
Fri Aug 2 19:26:09 UTC 2019


The rate filter is potentially opening fds in one thread while another
thread is processing a fork() in the plugin.  Although the file is not
open for long, we must either use atomic CLOEXEC to avoid fd leaks, or
degrade the thread model to SERIALIZE_ALL_REQUESTS.  However, the leak
is hard to observe (right now, it requires an out-of-tree plugin,
since the sh plugin is already at SERIALIZE_ALL_REQUESTS; and the
window where the file is open is already small).

The easiest solution would be the use of fopen("re"), but although
that is slated for future addition to POSIX [1], it is not currently
available in Haiku [2].  Thankfully, Haiku has O_CLOEXEC, so we can
use that.

[1] http://austingroupbugs.net/view.php?id=411
[2] https://dev.haiku-os.org/ticket/15219

This also fixes a minor bug where fclose() may have corrupted the
value of errno printed in nbdkit_debug after a failed getline().
Perhaps we could switch to low-level read() into a fixed buffer
instead of using fdopen()/readline(), but the patch already seemed
large enough.

Signed-off-by: Eric Blake <eblake at redhat.com>
---
 filters/rate/rate.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/filters/rate/rate.c b/filters/rate/rate.c
index f8dda0b0..dbd92ad6 100644
--- a/filters/rate/rate.c
+++ b/filters/rate/rate.c
@@ -34,6 +34,7 @@

 #include <config.h>

+#include <fcntl.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <stdint.h>
@@ -41,6 +42,7 @@
 #include <string.h>
 #include <time.h>
 #include <sys/time.h>
+#include <unistd.h>

 #include <pthread.h>

@@ -195,6 +197,7 @@ rate_close (void *handle)
 static void
 maybe_adjust (const char *file, struct bucket *bucket, pthread_mutex_t *lock)
 {
+  int fd;
   FILE *fp;
   ssize_t r;
   size_t len = 0;
@@ -204,16 +207,27 @@ maybe_adjust (const char *file, struct bucket *bucket, pthread_mutex_t *lock)

   if (!file) return;

-  fp = fopen (file, "r");
-  if (fp == NULL)
+  /* Alas, Haiku lacks fopen("re"), so we have to spell this out the
+   * long way. We require atomic CLOEXEC, in case the plugin is using
+   * fork() in a parallel thread model.
+   */
+  fd = open (file, O_CLOEXEC | O_RDONLY);
+  if (fd == -1)
     return; /* this is not an error */
+  fp = fdopen (fd, "r");
+  if (fp == NULL) {
+    nbdkit_debug ("fdopen: %s: %m", file);
+    close (fd);
+    return; /* unexpected, but treat it as a non-error */
+  }

   r = getline (&line, &len, fp);
-  fclose (fp);
   if (r == -1) {
     nbdkit_debug ("could not read rate file: %s: %m", file);
+    fclose (fp);
     return;
   }
+  fclose (fp);

   if (r > 0 && line[r-1] == '\n') line[r-1] = '\0';
   new_rate = nbdkit_parse_size (line);
-- 
2.20.1




More information about the Libguestfs mailing list