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 <[email protected]> --- 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 _______________________________________________ Libguestfs mailing list [email protected] https://www.redhat.com/mailman/listinfo/libguestfs
