Richard W.M. Jones
2016-May-21 12:14 UTC
[Libguestfs] [PATCH] umask: Use /proc/<PID>/status to read umask in Linux >= 4.7.
Since Linux 4.7, the process umask is available in /proc/<pid>/status. See: https://github.com/torvalds/linux/commit/3e42979e65dace1f9268dd5440e5ab096b8dee59 Use this value if available, else fall back to the existing codepath for Linux <= 4.6 and other Unix. --- src/umask.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 78 insertions(+), 8 deletions(-) diff --git a/src/umask.c b/src/umask.c index b8748e8..3f32337 100644 --- a/src/umask.c +++ b/src/umask.c @@ -18,12 +18,22 @@ /** * Return current umask in a thread-safe way. + * + * glibc documents, but does not actually implement, a "getumask(3)" + * call. + * + * We use C<Umask> from F</proc/I<PID>/status> for Linux E<ge> 4.7. + * For older Linux and other Unix, this file implements an expensive + * but thread-safe way to get the current process's umask. + * + * Thanks to: Josh Stone, Jiri Jaburek, Eric Blake. */ #include <config.h> #include <stdio.h> #include <stdlib.h> +#include <stdbool.h> #include <unistd.h> #include <fcntl.h> #include <errno.h> @@ -35,21 +45,81 @@ #include "guestfs.h" #include "guestfs-internal.h" +static int get_umask_from_proc (guestfs_h *g); +static int get_umask_from_fork (guestfs_h *g); + /** - * 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) { + int mask; + + mask = get_umask_from_proc (g); + if (mask == -1) + return -1; + if (mask >= 0) + return mask; + + return get_umask_from_fork (g); +} + +/** + * For Linux E<ge> 4.7 get the umask from F</proc/I<PID>/status>. + * + * On failure this returns C<-1>. However if we could not open the + * F</proc> file or find the C<Umask> entry in it, return C<-2> which + * causes the fallback path to run. + */ +static int +get_umask_from_proc (guestfs_h *g) +{ + FILE *fp; + CLEANUP_FREE char *path = NULL, *line = NULL; + size_t allocsize; + ssize_t len; + unsigned int mask; + bool found = false; + + path = safe_asprintf (g, "/proc/%d/status", getpid ()); + fp = fopen (path, "r"); + if (fp == NULL) { + if (errno == ENOENT || errno == ENOTDIR) + return -2; /* fallback */ + perrorf (g, "open: %s", path); + return -1; + } + + while ((len = getline (&line, &allocsize, fp)) != -1) { + if (len > 0 && line[len-1] == '\n') + line[--len] = '\0'; + + /* Looking for: "Umask: 0022" */ + if (sscanf (line, "Umask: %o", &mask) == 1) { + found = true; + break; + } + } + + if (fclose (fp) == -1) { + perrorf (g, "close: %s", path); + return -1; + } + + if (!found) + return -2; /* fallback */ + + return (int) mask; +} + +/** + * Fallback method of getting the umask using fork. + */ +static int +get_umask_from_fork (guestfs_h *g) +{ pid_t pid; int fd[2], r; int mask; -- 2.7.4
Pino Toscano
2016-May-23 08:49 UTC
Re: [Libguestfs] [PATCH] umask: Use /proc/<PID>/status to read umask in Linux >= 4.7.
On Saturday 21 May 2016 13:14:40 Richard W.M. Jones wrote:> Since Linux 4.7, the process umask is available in /proc/<pid>/status. > See: > https://github.com/torvalds/linux/commit/3e42979e65dace1f9268dd5440e5ab096b8dee59 > > Use this value if available, else fall back to the existing codepath > for Linux <= 4.6 and other Unix. > ---Sounds good -- just wondering whether the result of the inspection of the "status" file should be cached somehow, so following get_umask calls after the first one will use only the working method. A couple of notes follow.> src/umask.c | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 78 insertions(+), 8 deletions(-) > > diff --git a/src/umask.c b/src/umask.c > index b8748e8..3f32337 100644 > --- a/src/umask.c > +++ b/src/umask.c > @@ -18,12 +18,22 @@ > > /** > * Return current umask in a thread-safe way. > + * > + * glibc documents, but does not actually implement, a "getumask(3)" > + * call. > + * > + * We use C<Umask> from F</proc/I<PID>/status> for Linux E<ge> 4.7. > + * For older Linux and other Unix, this file implements an expensive > + * but thread-safe way to get the current process's umask. > + * > + * Thanks to: Josh Stone, Jiri Jaburek, Eric Blake. > */ > > #include <config.h> > > #include <stdio.h> > #include <stdlib.h> > +#include <stdbool.h> > #include <unistd.h> > #include <fcntl.h> > #include <errno.h> > @@ -35,21 +45,81 @@ > #include "guestfs.h" > #include "guestfs-internal.h" > > +static int get_umask_from_proc (guestfs_h *g); > +static int get_umask_from_fork (guestfs_h *g); > + > /** > - * 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) > { > + int mask; > + > + mask = get_umask_from_proc (g); > + if (mask == -1) > + return -1; > + if (mask >= 0) > + return mask; > + > + return get_umask_from_fork (g); > +} > + > +/** > + * For Linux E<ge> 4.7 get the umask from F</proc/I<PID>/status>. > + * > + * On failure this returns C<-1>. However if we could not open the > + * F</proc> file or find the C<Umask> entry in it, return C<-2> which > + * causes the fallback path to run. > + */ > +static int > +get_umask_from_proc (guestfs_h *g) > +{ > + FILE *fp;There's CLEANUP_FCLOSE, although it discards the error on fclose() (which could be ignored, anyway?).> + CLEANUP_FREE char *path = NULL, *line = NULL; > + size_t allocsize;This needs to be reset to 0, as documented for getline.> + ssize_t len; > + unsigned int mask; > + bool found = false; > + > + path = safe_asprintf (g, "/proc/%d/status", getpid ());This does not need a new buffer on heap, a short stack buffer like char path[sizeof ("/proc/%d/status") + sizeof (pid_t) * 3]; should be enough.> + fp = fopen (path, "r"); > + if (fp == NULL) { > + if (errno == ENOENT || errno == ENOTDIR) > + return -2; /* fallback */ > + perrorf (g, "open: %s", path); > + return -1; > + } > + > + while ((len = getline (&line, &allocsize, fp)) != -1) { > + if (len > 0 && line[len-1] == '\n') > + line[--len] = '\0'; > + > + /* Looking for: "Umask: 0022" */ > + if (sscanf (line, "Umask: %o", &mask) == 1) { > + found = true; > + break; > + } > + } > + > + if (fclose (fp) == -1) { > + perrorf (g, "close: %s", path); > + return -1; > + } > + > + if (!found) > + return -2; /* fallback */ > + > + return (int) mask; > +} > + > +/** > + * Fallback method of getting the umask using fork. > + */ > +static int > +get_umask_from_fork (guestfs_h *g) > +{ > pid_t pid; > int fd[2], r; > int mask; >-- Pino Toscano
Possibly Parallel Threads
- [PATCH libguestfs] launch: Implement a safer getumask.
- [PATCH v3 libguestfs] launch: Implement a safer getumask.
- [PATCH v2 libguestfs] launch: Implement a safer getumask.
- Re: [PATCH libguestfs] launch: Implement a safer getumask.
- Re: [PATCH v3 libguestfs] launch: Implement a safer getumask.