The current implementation of getumask involves writing a file with mode 0777 and then testing what mode was created by the kernel. This doesn't work properly if the user set a per-mount umask (or fmask/ dmask). This alternative method was suggested by Josh Stone. By forking, we can use the thread-unsafe method (calling umask) and pass the result back over a pipe. This change also fixes another problem: mode_t is unsigned, so cannot be used to return an error indication (ie. -1). Return a plain int instead. Thanks: Josh Stone, Jiri Jaburek, Eric Blake. --- docs/C_SOURCE_FILES | 1 + po/POTFILES | 1 + src/Makefile.am | 1 + src/guestfs-internal.h | 3 ++ src/launch.c | 41 ++---------------- src/umask.c | 110 +++++++++++++++++++++++++++++++++++++++++++++++++ 6 files changed, 120 insertions(+), 37 deletions(-) create mode 100644 src/umask.c diff --git a/docs/C_SOURCE_FILES b/docs/C_SOURCE_FILES index a477744..f828911 100644 --- a/docs/C_SOURCE_FILES +++ b/docs/C_SOURCE_FILES @@ -269,6 +269,7 @@ src/structs-free.c src/structs-print.c src/test-utils.c src/tmpdirs.c +src/umask.c src/utils.c src/whole-file.c test-tool/test-tool.c diff --git a/po/POTFILES b/po/POTFILES index 9e4d9cc..23599b9 100644 --- a/po/POTFILES +++ b/po/POTFILES @@ -359,6 +359,7 @@ src/structs-free.c src/structs-print.c src/test-utils.c src/tmpdirs.c +src/umask.c src/utils.c src/whole-file.c test-tool/test-tool.c diff --git a/src/Makefile.am b/src/Makefile.am index 3b4cd10..34c4fa6 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -130,6 +130,7 @@ libguestfs_la_SOURCES = \ structs-copy.c \ structs-free.c \ tmpdirs.c \ + umask.c \ whole-file.c \ libguestfs.syms diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index 61f384c..bf107d0 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -915,4 +915,7 @@ void guestfs_int_init_unix_backend (void) __attribute__((constructor)); /* guid.c */ extern int guestfs_int_validate_guid (const char *); +/* umask.c */ +extern int guestfs_int_getumask (guestfs_h *g); + #endif /* GUESTFS_INTERNAL_H_ */ diff --git a/src/launch.c b/src/launch.c index a4326fe..460ed29 100644 --- a/src/launch.c +++ b/src/launch.c @@ -50,8 +50,6 @@ static struct backend { const struct backend_ops *ops; } *backends = NULL; -static mode_t get_umask (guestfs_h *g); - int guestfs_impl_launch (guestfs_h *g) { @@ -75,6 +73,7 @@ guestfs_impl_launch (guestfs_h *g) guestfs_version (g); struct backend *b; CLEANUP_FREE char *backend = guestfs_get_backend (g); + int mask; debug (g, "launch: program=%s", g->program); if (STRNEQ (g->identifier, "")) @@ -87,7 +86,9 @@ guestfs_impl_launch (guestfs_h *g) debug (g, "launch: backend=%s", backend); debug (g, "launch: tmpdir=%s", g->tmpdir); - debug (g, "launch: umask=0%03o", get_umask (g)); + mask = guestfs_int_getumask (g); + if (mask >= 0) + debug (g, "launch: umask=0%03o", (unsigned) mask); debug (g, "launch: euid=%ju", (uintmax_t) geteuid ()); } @@ -475,40 +476,6 @@ guestfs_int_create_socketname (guestfs_h *g, const char *filename, } /** - * glibc documents, but does not actually implement, a L<getumask(3)> - * call. - * - * This function implements a thread-safe way to get the umask. Note - * this is only called when C<g-E<gt>verbose> is true and after - * C<g-E<gt>tmpdir> has been created. - */ -static mode_t -get_umask (guestfs_h *g) -{ - mode_t ret; - int fd; - struct stat statbuf; - CLEANUP_FREE char *filename = safe_asprintf (g, "%s/umask-check", g->tmpdir); - - fd = open (filename, O_WRONLY|O_CREAT|O_TRUNC|O_NOCTTY|O_CLOEXEC, 0777); - if (fd == -1) - return -1; - - if (fstat (fd, &statbuf) == -1) { - close (fd); - return -1; - } - - close (fd); - - ret = statbuf.st_mode; - ret &= 0777; - ret = ret ^ 0777; - - return ret; -} - -/** * When the library is loaded, each backend calls this function to * register itself in a global list. */ diff --git a/src/umask.c b/src/umask.c new file mode 100644 index 0000000..3fc7bc6 --- /dev/null +++ b/src/umask.c @@ -0,0 +1,110 @@ +/* libguestfs + * Copyright (C) 2016 Red Hat Inc. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/** + * Return current umask in a thread-safe way. + */ + +#include <config.h> + +#include <stdio.h> +#include <stdlib.h> +#include <unistd.h> +#include <fcntl.h> +#include <errno.h> +#include <sys/stat.h> +#include <sys/types.h> +#include <sys/wait.h> + +#include "ignore-value.h" + +#include "guestfs.h" +#include "guestfs-internal.h" + +/** + * glibc documents, but does not actually implement, a L<getumask(3)> + * call. + * + * This function implements an expensive, but thread-safe way to get + * the current process's umask. + * + * Returns the current process's umask. On failure, returns C<-1> and + * sets the error in the guestfs handle. + * + * Thanks to: Josh Stone, Jiri Jaburek, Eric Blake. + */ +int +guestfs_int_getumask (guestfs_h *g) +{ + pid_t pid; + int fd[2], r; + int mask; + int status; + + r = pipe2 (fd, O_CLOEXEC); + if (r == -1) { + perrorf (g, "pipe2"); + return -1; + } + + pid = fork (); + if (pid == -1) { + perrorf (g, "fork"); + close (fd[0]); + close (fd[1]); + return -1; + } + if (pid == 0) { + /* The child process must ONLY call async-safe functions. */ + close (fd[0]); + + /* umask can't fail. */ + mask = umask (0); + + if (write (fd[1], &mask, sizeof mask) != sizeof mask) + _exit (EXIT_FAILURE); + if (close (fd[1]) == -1) + _exit (EXIT_FAILURE); + + _exit (EXIT_SUCCESS); + } + + /* Parent. */ + close (fd[1]); + + /* Read the umask. */ + if (read (fd[0], &mask, sizeof mask) != sizeof mask) { + perrorf (g, "read"); + close (fd[0]); + return -1; + } + close (fd[0]); + + again: + if (waitpid (pid, &status, 0) == -1) { + if (errno == EINTR) goto again; + perrorf (g, "waitpid"); + return -1; + } + else if (!WIFEXITED (status) || WEXITSTATUS (status) != 0) { + guestfs_int_external_command_failed (g, status, "umask", NULL); + return -1; + } + + return mask; +} -- 2.7.4