]> Zhao Yanbai Git Server - minix.git/commitdiff
vfs: fix null deref, pfs: add fchmod() 08/508/5
authorBen Gras <ben@minix3.org>
Mon, 15 Apr 2013 17:44:19 +0000 (17:44 +0000)
committerBen Gras <ben@minix3.org>
Fri, 19 Apr 2013 15:06:56 +0000 (17:06 +0200)
. 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

servers/pfs/misc.c
servers/pfs/proto.h
servers/pfs/table.c
servers/vfs/protect.c
test/test56.c

index 541bb4a9258cb57e288d7a2bbf2fb43c3fed9346..296e46fdbf0d6aa70b50ea5dbb772cdc9c057497 100644 (file)
@@ -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;
+}
index c1c801d3ca4951f6bf398974e27a60d729bcbc9d..a268be29ad4f13ce1b7d74d811db8909a8990596 100644 (file)
@@ -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);
index 1eba29ec6d3984d0dfb928c371f4a265aba5fd51..ff3e62da395bbdf61f68b54d4841a19844f64ab2 100644 (file)
@@ -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   */
index ce71cee5e363e52cb15e60fa29e3e965b438edc3..4f76bc9e2121ea2b4e5480ce2085b845666e8c32 100644 (file)
@@ -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);
 }
index bbf17e056b9868056ec307554145544f9feb4a01..0b1b3197aa3ba7c233eb200b34eaa8ed746aa5ad 100644 (file)
@@ -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 */