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

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(a)redhat.com&gt; --- 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