[Libguestfs] [nbdkit PATCH 5/8] cow, cache: Better mkostemp fallback

Haiku is seriously behind in adding support for atomic CLOEXEC; in 2018, we added fallback support since it does not yet provide mkostemp as requested by an upcoming POSIX addition [1]. However, the fallback we added is rather brain-dead - if mkstemp() fails, we blindly call fcntl(-1) (which probably changes the errno later reported against "mkostemp: %s: %m"); and although it is historically unlikely that any other bits will be set, F_SETFD should be used in a read-modify-write pattern rather than a blind overwrite pattern. The fact that our fallback is non-atomic is not a problem in practice, since we are only using this code during .load (where we are still more-or-less single-threaded), rather than while another thread may be actively inside a plugin (such as sh) using fork(). [1] http://austingroupbugs.net/view.php?id=411 Fixes: 60076fbcfd Fixes: b962272a56 Signed-off-by: Eric Blake <eblake(a)redhat.com&gt; --- filters/cache/blk.c | 19 ++++++++++++++++++- filters/cow/blk.c | 19 ++++++++++++++++++- 2 files changed, 36 insertions(+), 2 deletions(-) diff --git a/filters/cache/blk.c b/filters/cache/blk.c index cf7145d8..1fa9ea43 100644 --- a/filters/cache/blk.c +++ b/filters/cache/blk.c @@ -109,8 +109,25 @@ blk_init (void) #ifdef HAVE_MKOSTEMP fd = mkostemp (template, O_CLOEXEC); #else + /* Not atomic, but this is only invoked during .load, so the race + * won't affect any plugin actions trying to fork + */ fd = mkstemp (template); - fcntl (fd, F_SETFD, FD_CLOEXEC); + if (fd >= 0) { + int f = fcntl (fd, F_GETFD); + if (f == -1) { + nbdkit_error ("fcntl F_GETFD: %s: %m", template); + close (fd); + unlink (template); + return -1; + } + if (fcntl (fd, F_SETFD, f | FD_CLOEXEC) == -1) { + nbdkit_error ("fcntl F_SETFD: %s: %m", template); + close (fd); + unlink (template); + return -1; + } + } #endif if (fd == -1) { nbdkit_error ("mkostemp: %s: %m", tmpdir); diff --git a/filters/cow/blk.c b/filters/cow/blk.c index be43f2ff..12ad7820 100644 --- a/filters/cow/blk.c +++ b/filters/cow/blk.c @@ -114,8 +114,25 @@ blk_init (void) #ifdef HAVE_MKOSTEMP fd = mkostemp (template, O_CLOEXEC); #else + /* Not atomic, but this is only invoked during .load, so the race + * won't affect any plugin actions trying to fork + */ fd = mkstemp (template); - fcntl (fd, F_SETFD, FD_CLOEXEC); + if (fd >= 0) { + int f = fcntl (fd, F_GETFD); + if (f == -1) { + nbdkit_error ("fcntl F_GETFD: %s: %m", template); + close (fd); + unlink (template); + return -1; + } + if (fcntl (fd, F_SETFD, f | FD_CLOEXEC) == -1) { + nbdkit_error ("fcntl F_SETFD: %s: %m", template); + close (fd); + unlink (template); + return -1; + } + } #endif if (fd == -1) { nbdkit_error ("mkostemp: %s: %m", tmpdir); -- 2.20.1