[Libguestfs] [PATCH] umask: Use /proc/<PID>/status to read umask in Linux >= 4.7.

[Pino Toscano]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://listman.redhat.com/archives/libguestfs/attachments/20160523/11d0ab84/attachment.sig>