On 04/13/2016 03:43 PM, Richard W.M. Jones wrote:
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. ---
+/** + * 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]); + + mask = umask (0); + if (mask == -1) { + perror ("umask");
perror() is NOT async-safe (it manipulates stdio, and could permanently wedgelock the child if you happened to fork() while some other thread was also using stdio). Safe error reporting in a fork()d child is basically impossible; the best you can do is write() something back to the parent and let the parent do the error reporting. :(
+ _exit (EXIT_FAILURE); + } + + if (write (fd[1], &mask, sizeof mask) != sizeof mask) { + perror ("write"); + _exit (EXIT_FAILURE); + } + if (close (fd[1]) == -1) { + perror ("close");
Again, more bad use of perror.
+ _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]); + + if (waitpid (pid, &status, 0) == -1) { + perrorf (g, "waitpid"); + return -1; + }
Doesn't this need to loop on EINTR, rather than immediately giving up?
+ else if (!WIFEXITED (status) || WEXITSTATUS (status) != 0) { + guestfs_int_external_command_failed (g, status, "umask", NULL); + return -1; + } + + return mask; +}
-- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org