[Libguestfs] [nbdkit PATCH 7/8] filters: Set CLOEXEC on files opened during .config
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(a)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