From: Ben Gras <ben@minix3.org>
Date: Mon, 15 Apr 2013 17:44:19 +0000 (+0000)
Subject: vfs: fix null deref, pfs: add fchmod()
X-Git-Tag: v3.3.0~1018
X-Git-Url: http://zhaoyanbai.com/repos/%22http:/www.isc.org/icons/sign.html?a=commitdiff_plain;h=072d916c1cf078f88c1beae2d24aba34ff259de4;p=minix.git

vfs: fix null deref, pfs: add fchmod()

	. vfs read_only() assumes vnode->v_vmnt is non-NULL, but it can
	  be NULL sometimes
	. e.g. fchmod() on UDS triggered NULL deref; add a check and
	  add REQ_CHMOD to pfs so unix domain sockets can be fchmod()ded
	. add to test56

Change-Id: I83c840f101b647516897cc99fcf472116d762012
---

diff --git a/servers/pfs/misc.c b/servers/pfs/misc.c
index 541bb4a92..296e46fdb 100644
--- a/servers/pfs/misc.c
+++ b/servers/pfs/misc.c
@@ -1,4 +1,5 @@
 #include "fs.h"
+#include "inode.h"
 
 
 /*===========================================================================*
@@ -10,3 +11,18 @@ int fs_sync(message *fs_m_in, message *fs_m_out)
 
   return(OK);		/* sync() can't fail */
 }
+
+/*===========================================================================*
+ *                             fs_chmod					     *
+ *===========================================================================*/
+int fs_chmod(message *fs_m_in, message *fs_m_out)
+{
+  struct inode *rip;  /* target inode */
+  mode_t mode = (mode_t) fs_m_in->REQ_MODE;
+
+  if( (rip = find_inode(fs_m_in->REQ_INODE_NR)) == NULL) return(EINVAL);
+  get_inode(rip->i_dev, rip->i_num);	/* mark inode in use */
+  rip->i_mode = (rip->i_mode & ~ALL_MODES) | (mode & ALL_MODES);
+  put_inode(rip);			/* release the inode */
+  return OK;
+}
diff --git a/servers/pfs/proto.h b/servers/pfs/proto.h
index c1c801d3c..a268be29a 100644
--- a/servers/pfs/proto.h
+++ b/servers/pfs/proto.h
@@ -38,6 +38,7 @@ void reply(endpoint_t who, message *m_out);
 
 /* misc.c */
 int fs_sync(message *fs_m_in, message *fs_m_out);
+int fs_chmod(message *fs_m_in, message *fs_m_out);
 
 /* mount.c */
 int fs_unmount(message *fs_m_in, message *fs_m_out);
diff --git a/servers/pfs/table.c b/servers/pfs/table.c
index 1eba29ec6..ff3e62da3 100644
--- a/servers/pfs/table.c
+++ b/servers/pfs/table.c
@@ -19,7 +19,7 @@ int (*fs_call_vec[])(message *fs_m_in, message *fs_m_out) = {
         no_sys,             /* 3   */
         fs_ftrunc,          /* 4   */
         no_sys,             /* 5   */
-	no_sys,             /* 6   */
+	fs_chmod,           /* 6   */
         no_sys,             /* 7   */
         fs_stat,            /* 8   */
         no_sys,             /* 9   */
diff --git a/servers/vfs/protect.c b/servers/vfs/protect.c
index ce71cee5e..4f76bc9e2 100644
--- a/servers/vfs/protect.c
+++ b/servers/vfs/protect.c
@@ -11,6 +11,7 @@
 #include "fs.h"
 #include <sys/stat.h>
 #include <unistd.h>
+#include <assert.h>
 #include <minix/callnr.h>
 #include "file.h"
 #include "fproc.h"
@@ -62,9 +63,12 @@ int do_chmod(message *UNUSED(m_out))
 	/* File is already opened; get a pointer to vnode from filp. */
 	if ((flp = get_filp(rfd, VNODE_WRITE)) == NULL) return(err_code);
 	vp = flp->filp_vno;
+        assert(vp);
 	dup_vnode(vp);
   }
 
+  assert(vp);
+
   /* Only the owner or the super_user may change the mode of a file.
    * No one may change the mode of a file on a read-only file system.
    */
@@ -304,5 +308,6 @@ struct vnode *vp;		/* ptr to inode whose file sys is to be cked */
 /* Check to see if the file system on which the inode 'ip' resides is mounted
  * read only.  If so, return EROFS, else return OK.
  */
-  return((vp->v_vmnt->m_flags & VMNT_READONLY) ? EROFS : OK);
+  assert(vp);
+  return(vp->v_vmnt && (vp->v_vmnt->m_flags & VMNT_READONLY) ? EROFS : OK);
 }
diff --git a/test/test56.c b/test/test56.c
index bbf17e056..0b1b3197a 100644
--- a/test/test56.c
+++ b/test/test56.c
@@ -2852,6 +2852,39 @@ void test_select()
 
 }
 
+void test_fchmod()
+{
+	int socks[2];
+	struct stat st1, st2;
+
+	if (socketpair(AF_UNIX, SOCK_STREAM, 0, socks) < 0) {
+		test_fail("Can't open socket pair.");
+	}
+
+	if (fstat(socks[0], &st1) < 0 || fstat(socks[1], &st2) < 0) {
+		test_fail("fstat failed.");
+	}
+
+	if ((st1.st_mode & (S_IRUSR|S_IWUSR)) == S_IRUSR &&
+		(st2.st_mode & (S_IRUSR|S_IWUSR)) == S_IWUSR) {
+		test_fail("fstat failed.");
+	}
+
+	if (fchmod(socks[0], S_IRUSR) < 0 ||
+                    fstat(socks[0], &st1) < 0 ||
+                    (st1.st_mode & (S_IRUSR|S_IWUSR)) != S_IRUSR) {
+		test_fail("fchmod/fstat mode set/check failed (1).");
+	}
+
+	if (fchmod(socks[1], S_IWUSR) < 0 || fstat(socks[1], &st2) < 0 ||
+                    (st2.st_mode & (S_IRUSR|S_IWUSR)) != S_IWUSR) {
+		test_fail("fchmod/fstat mode set/check failed (2).");
+	}
+
+	close(socks[0]);
+	close(socks[1]);
+}
+
 int main(int argc, char *argv[])
 {
 	int i;
@@ -2892,6 +2925,7 @@ int main(int argc, char *argv[])
 	test_scm_credentials();
 	test_fd_passing();
 	test_select();
+	test_fchmod();
 	quit();
 
 	return -1;	/* we should never get here */