[Libguestfs] [PATCH] mknod: filter modes in mkfifo, mknod_b, mknod_c (RHBZ#1182463).

[Richard W.M. Jones]

On Thu, Jan 15, 2015 at 02:44:58PM +0100, Pino Toscano wrote:
> Since mkfifo, mknod_b, and mknod_c add the correct file type to the
> modes of the resulting file, make sure the specified mode contains only
> permissions bits.
> ---
>  daemon/mknod.c       | 15 +++++++++++++++
>  generator/actions.ml | 21 ++++++++++++++++++---
>  2 files changed, 33 insertions(+), 3 deletions(-)
> 
> diff --git a/daemon/mknod.c b/daemon/mknod.c
> index 7f71210..9af8701 100644
> --- a/daemon/mknod.c
> +++ b/daemon/mknod.c
> @@ -38,6 +38,15 @@ optgroup_mknod_available (void)
>    return 1;
>  }
>  
> +#define CHECK_MODE \
> +  do { \
> +    if ((mode & ~07777) != 0) { \
> +      reply_with_error ("%o: mode must specify only file permission bits", \
> +                        (unsigned int) mode); \
> +      return -1; \
> +    } \
> +  } while (0)
> +
>  int
>  do_mknod (int mode, int devmajor, int devminor, const char *path)
>  {
> @@ -63,18 +72,24 @@ do_mknod (int mode, int devmajor, int devminor, const char *path)
>  int
>  do_mkfifo (int mode, const char *path)
>  {
> +  CHECK_MODE;
> +
>    return do_mknod (mode | S_IFIFO, 0, 0, path);
>  }
>  
>  int
>  do_mknod_b (int mode, int devmajor, int devminor, const char *path)
>  {
> +  CHECK_MODE;
> +
>    return do_mknod (mode | S_IFBLK, devmajor, devminor, path);
>  }
>  
>  int
>  do_mknod_c (int mode, int devmajor, int devminor, const char *path)
>  {
> +  CHECK_MODE;
> +
>    return do_mknod (mode | S_IFCHR, devmajor, devminor, path);
>  }
>  
> diff --git a/generator/actions.ml b/generator/actions.ml
> index 96a9dd6..c48ad1a 100644
> --- a/generator/actions.ml
> +++ b/generator/actions.ml
> @@ -6173,7 +6173,9 @@ The mode actually set is affected by the umask." };
>        InitScratchFS, Always, TestResult (
>          [["mkfifo"; "0o777"; "/mkfifo"];
>           ["stat"; "/mkfifo"]],
> -        "S_ISFIFO (ret->mode) && (ret->mode & 0777) == 0755"), []
> +        "S_ISFIFO (ret->mode) && (ret->mode & 0777) == 0755"), [];
> +      InitScratchFS, Always, TestLastFail (
> +        [["mkfifo"; "0o20777"; "/mkfifo-2"]]), [];
>      ];
>      shortdesc = "make FIFO (named pipe)";
>      longdesc = "\
> @@ -6181,6 +6183,9 @@ This call creates a FIFO (named pipe) called C<path> with
>  mode C<mode>.  It is just a convenient wrapper around
>  C<guestfs_mknod>.
>  
> +Unlike with C<guestfs_mknod>, C<mode> B<must> contain only permissions
> +bits.
> +
>  The mode actually set is affected by the umask." };
>  
>    { defaults with
> @@ -6192,7 +6197,9 @@ The mode actually set is affected by the umask." };
>        InitScratchFS, Always, TestResult (
>          [["mknod_b"; "0o777"; "99"; "66"; "/mknod_b"];
>           ["stat"; "/mknod_b"]],
> -        "S_ISBLK (ret->mode) && (ret->mode & 0777) == 0755"), []
> +        "S_ISBLK (ret->mode) && (ret->mode & 0777) == 0755"), [];
> +      InitScratchFS, Always, TestLastFail (
> +        [["mknod_b"; "0o10777"; "99"; "66"; "/mknod_b-2"]]), [];
>      ];
>      shortdesc = "make block device node";
>      longdesc = "\
> @@ -6200,6 +6207,9 @@ This call creates a block device node called C<path> with
>  mode C<mode> and device major/minor C<devmajor> and C<devminor>.
>  It is just a convenient wrapper around C<guestfs_mknod>.
>  
> +Unlike with C<guestfs_mknod>, C<mode> B<must> contain only permissions
> +bits.
> +
>  The mode actually set is affected by the umask." };
>  
>    { defaults with
> @@ -6211,7 +6221,9 @@ The mode actually set is affected by the umask." };
>        InitScratchFS, Always, TestResult (
>          [["mknod_c"; "0o777"; "99"; "66"; "/mknod_c"];
>           ["stat"; "/mknod_c"]],
> -        "S_ISCHR (ret->mode) && (ret->mode & 0777) == 0755"), []
> +        "S_ISCHR (ret->mode) && (ret->mode & 0777) == 0755"), [];
> +      InitScratchFS, Always, TestLastFail (
> +        [["mknod_c"; "0o20777"; "99"; "66"; "/mknod_c-2"]]), [];
>      ];
>      shortdesc = "make char device node";
>      longdesc = "\
> @@ -6219,6 +6231,9 @@ This call creates a char device node called C<path> with
>  mode C<mode> and device major/minor C<devmajor> and C<devminor>.
>  It is just a convenient wrapper around C<guestfs_mknod>.
>  
> +Unlike with C<guestfs_mknod>, C<mode> B<must> contain only permissions
> +bits.
> +
>  The mode actually set is affected by the umask." };
>  
>    { defaults with
> -- 
> 1.9.3

ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html