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