Improved checking, documentation of modes (RHBZ#582901, RHBZ#582929).

chmod: Disallow negative mode, document mode affected by umask.

mkdir-mode: Disallow negative mode, document that filesystems
may interpret the mode in different ways.

mknod: Disallow negative mode, document mode affected by umask.

umask: Check the range of umask mask value carefully.

5 files changed, 42 insertions, 6 deletions

diff --git a/daemon/dir.c b/daemon/dir.c
index 205139b8..3a4647cd 100644
--- a/daemon/dir.c
+++ b/daemon/dir.c
@@ -104,6 +104,11 @@ do_mkdir_mode (const char *path, int mode)
 {
   int r;
 
+  if (mode < 0) {
+    reply_with_error ("%s: mode is negative", path);
+    return -1;
+  }
+
   CHROOT_IN;
   r = mkdir (path, mode);
   CHROOT_OUT;
diff --git a/daemon/file.c b/daemon/file.c
index dbdbbaae..7a0f8f92 100644
--- a/daemon/file.c
+++ b/daemon/file.c
@@ -198,6 +198,11 @@ do_chmod (int mode, const char *path)
 {
   int r;
 
+  if (mode < 0) {
+    reply_with_error ("%s: mode is negative", path);
+    return -1;
+  }
+
   CHROOT_IN;
   r = chmod (path, mode);
   CHROOT_OUT;
diff --git a/daemon/mknod.c b/daemon/mknod.c
index 50865a62..2458a851 100644
--- a/daemon/mknod.c
+++ b/daemon/mknod.c
@@ -51,6 +51,11 @@ do_mknod (int mode, int devmajor, int devminor, const char *path)
 #ifdef HAVE_MKNOD
   int r;
 
+  if (mode < 0) {
+    reply_with_error ("%s: mode is negative", path);
+    return -1;
+  }
+
   CHROOT_IN;
   r = mknod (path, mode, makedev (devmajor, devminor));
   CHROOT_OUT;
diff --git a/daemon/umask.c b/daemon/umask.c
index a9ddeab0..db4a2f2f 100644
--- a/daemon/umask.c
+++ b/daemon/umask.c
@@ -35,6 +35,11 @@ do_umask (int mask)
 {
   int r;
 
+  if (mask < 0 || mask > 0777) {
+    reply_with_error ("0%o: mask negative or out of range", mask);
+    return -1;
+  }
+
   r = umask (mask);
 
   if (r == -1) {
diff --git a/src/generator.ml b/src/generator.ml
index 61323a92..88698420 100755
--- a/src/generator.ml
+++ b/src/generator.ml
@@ -1390,7 +1390,9 @@ numeric modes are supported.
 
 I<Note>: When using this command from guestfish, C<mode>
 by default would be decimal, unless you prefix it with
-C<0> to get octal, ie. use C<0700> not C<700>.");
+C<0> to get octal, ie. use C<0700> not C<700>.
+
+The mode actually set is affected by the umask.");
 
   ("chown", (RErr, [Int "owner"; Int "group"; Pathname "path"]), 35, [],
    [], (* XXX Need stat command to test *)
@@ -2985,7 +2987,9 @@ named pipes (FIFOs).
 The C<mode> parameter should be the mode, using the standard
 constants.  C<devmajor> and C<devminor> are the
 device major and minor numbers, only used when creating block
-and character special devices.");
+and character special devices.
+
+The mode actually set is affected by the umask.");
 
   ("mkfifo", (RErr, [Int "mode"; Pathname "path"]), 134, [Optional "mknod"],
    [InitBasicFS, Always, TestOutputStruct (
@@ -2995,7 +2999,9 @@ and character special devices.");
    "\
 This call creates a FIFO (named pipe) called C<path> with
 mode C<mode>.  It is just a convenient wrapper around
-C<guestfs_mknod>.");
+C<guestfs_mknod>.
+
+The mode actually set is affected by the umask.");
 
   ("mknod_b", (RErr, [Int "mode"; Int "devmajor"; Int "devminor"; Pathname "path"]), 135, [Optional "mknod"],
    [InitBasicFS, Always, TestOutputStruct (
@@ -3005,7 +3011,9 @@ C<guestfs_mknod>.");
    "\
 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>.");
+It is just a convenient wrapper around C<guestfs_mknod>.
+
+The mode actually set is affected by the umask.");
 
   ("mknod_c", (RErr, [Int "mode"; Int "devmajor"; Int "devminor"; Pathname "path"]), 136, [Optional "mknod"],
    [InitBasicFS, Always, TestOutputStruct (
@@ -3015,7 +3023,9 @@ It is just a convenient wrapper around C<guestfs_mknod>.");
    "\
 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>.");
+It is just a convenient wrapper around C<guestfs_mknod>.
+
+The mode actually set is affected by the umask.");
 
   ("umask", (RInt "oldmask", [Int "mask"]), 137, [FishOutput FishOutputOctal],
    [InitEmpty, Always, TestOutputInt (
@@ -3879,7 +3889,13 @@ C<*secs> field is ignored in this case).");
    "create a directory with a particular mode",
    "\
 This command creates a directory, setting the initial permissions
-of the directory to C<mode>.  See also C<guestfs_mkdir>.");
+of the directory to C<mode>.
+
+For common Linux filesystems, the actual mode which is set will
+be C<mode & ~umask & 01777>.  Non-native-Linux filesystems may
+interpret the mode in other ways.
+
+See also C<guestfs_mkdir>, C<guestfs_umask>");
 
   ("lchown", (RErr, [Int "owner"; Int "group"; Pathname "path"]), 203, [],
    [], (* XXX *)