[NaCk SDK] nacl_io: Don't assume ~S_IFMT are the mode bits.

Define a new macro S_MODEBITS to represent the
mode bits settable by chmod/fchmod.

Add test to verify that file type cannot be changed
with chmod.

Review URL: https://codereview.chromium.org/671513002

Cr-Commit-Position: refs/heads/master@{#300341}

native_client_sdk/src/libraries/nacl_io/dir_node.cc

@@ -70,7 +70,7 @@ Error DirNode::GetDents(size_t offs,
 
 Error DirNode::Fchmod(mode_t mode) {
   AUTO_LOCK(node_lock_);
-  SetMode(mode & ~S_IFMT);
+  SetMode(mode);
   return 0;
 }
 

native_client_sdk/src/libraries/nacl_io/fusefs/fuse_fs.cc

@@ -90,7 +90,7 @@ Error FuseFs::OpenWithMode(const Path& path, int open_flags, mode_t mode,
       if (result < 0)
         return -result;
 
-      if ((statbuf.st_mode & S_IFMT) == S_IFDIR) {
+      if (S_ISDIR(statbuf.st_mode)) {
         // This is a directory. Don't try to open, just create a new node with
         // this path.
         ScopedNode node(new DirFuseFsNode(this, fuse_ops_, fi, path_cstr));
@@ -102,7 +102,7 @@ Error FuseFs::OpenWithMode(const Path& path, int open_flags, mode_t mode,
         return 0;
       }
       // Get mode.
-      mode = statbuf.st_mode & ~S_IFMT;
+      mode = statbuf.st_mode & S_MODEBITS;
     }
 
     // Existing file.
@@ -189,7 +189,7 @@ Error FuseFs::Remove(const Path& path) {
 
   node.reset();
 
-  if ((statbuf.st_mode & S_IFMT) == S_IFDIR) {
+  if (S_ISDIR(statbuf.st_mode)) {
     return Rmdir(path);
   } else {
     return Unlink(path);

native_client_sdk/src/libraries/nacl_io/kernel_object.cc

@@ -162,7 +162,7 @@ mode_t KernelObject::GetUmask() {
 mode_t KernelObject::SetUmask(mode_t newmask) {
   AUTO_LOCK(umask_lock_);
   mode_t oldmask = umask_;
-  umask_ = newmask & 0777;
+  umask_ = newmask & S_MODEBITS;
   return oldmask;
 }
 

native_client_sdk/src/libraries/nacl_io/kernel_proxy.cc

@@ -209,7 +209,7 @@ int KernelProxy::open_resource(const char* path) {
 int KernelProxy::open(const char* path, int open_flags, mode_t mode) {
   ScopedFilesystem fs;
   ScopedNode node;
-  mode_t mask = ~GetUmask();
+  mode_t mask = ~GetUmask() & S_MODEBITS;
 
   Error error = AcquireFsAndNode(path, open_flags, mode & mask, &fs, &node);
   if (error) {
@@ -372,7 +372,8 @@ int KernelProxy::mkdir(const char* path, mode_t mode) {
     return -1;
   }
 
-  error = fs->Mkdir(rel, mode & ~GetUmask());
+  mode_t mask = ~GetUmask() & S_MODEBITS;
+  error = fs->Mkdir(rel, mode & mask);
   if (error) {
     errno = error;
     return -1;
@@ -878,7 +879,7 @@ int KernelProxy::fchmod(int fd, mode_t mode) {
     return -1;
   }
 
-  error = handle->node()->Fchmod(mode);
+  error = handle->node()->Fchmod(mode & S_MODEBITS);
   if (error) {
     errno = error;
     return -1;
@@ -1189,7 +1190,7 @@ int KernelProxy::sigaction(int signum,
 }
 
 mode_t KernelProxy::umask(mode_t mask) {
-  return SetUmask(mask);
+  return SetUmask(mask & S_MODEBITS);
 }
 
 #ifdef PROVIDES_SOCKET_API

native_client_sdk/src/libraries/nacl_io/memfs/mem_fs_node.cc

@@ -135,7 +135,7 @@ Error MemFsNode::Futimens(const struct timespec times[2]) {
 
 Error MemFsNode::Fchmod(mode_t mode) {
   AUTO_LOCK(node_lock_);
-  SetMode(mode & ~S_IFMT);
+  SetMode(mode);
   return 0;
 }
 

native_client_sdk/src/libraries/nacl_io/node.cc

@@ -189,7 +189,7 @@ int Node::GetLinks() {
 }
 
 int Node::GetMode() {
-  return stat_.st_mode & ~S_IFMT;
+  return stat_.st_mode & S_MODEBITS;
 }
 
 Error Node::GetSize(off_t* out_size) {
@@ -208,8 +208,8 @@ void Node::SetType(int type) {
 }
 
 void Node::SetMode(int mode) {
-  assert((mode & S_IFMT) == 0);
-  stat_.st_mode &= S_IFMT;
+  assert((mode & ~S_MODEBITS) == 0);
+  stat_.st_mode &= ~S_MODEBITS;
   stat_.st_mode |= mode;
 }
 

native_client_sdk/src/libraries/nacl_io/node.h

@@ -22,6 +22,7 @@
 #define S_IRALL (S_IRUSR | S_IRGRP | S_IROTH)
 #define S_IWALL (S_IWUSR | S_IWGRP | S_IWOTH)
 #define S_IXALL (S_IXUSR | S_IXGRP | S_IXOTH)
+#define S_MODEBITS 07777
 
 namespace nacl_io {
 

native_client_sdk/src/tests/nacl_io_test/fuse_fs_test.cc

@@ -289,8 +289,8 @@ TEST_F(FuseFsTest, CreateWithMode) {
   ASSERT_EQ(0, fs_.OpenWithMode(Path("/hello"),
                                 O_RDWR | O_CREAT, 0723, &node));
   EXPECT_EQ(0, node->GetStat(&statbuf));
-  EXPECT_EQ(S_IFREG, statbuf.st_mode & S_IFMT);
-  EXPECT_EQ(0723, statbuf.st_mode & ~S_IFMT);
+  EXPECT_TRUE(S_ISREG(statbuf.st_mode));
+  EXPECT_EQ(0723, statbuf.st_mode & S_MODEBITS);
 }
 
 TEST_F(FuseFsTest, CreateAndWrite) {
@@ -317,20 +317,20 @@ TEST_F(FuseFsTest, GetStat) {
 
   ASSERT_EQ(0, fs_.Open(Path("/hello"), O_RDONLY, &node));
   EXPECT_EQ(0, node->GetStat(&statbuf));
-  EXPECT_EQ(S_IFREG, statbuf.st_mode & S_IFMT);
-  EXPECT_EQ(0666, statbuf.st_mode & ~S_IFMT);
+  EXPECT_TRUE(S_ISREG(statbuf.st_mode));
+  EXPECT_EQ(0666, statbuf.st_mode & S_MODEBITS);
   EXPECT_EQ(strlen(hello_world), statbuf.st_size);
 
   ASSERT_EQ(0, fs_.Open(Path("/"), O_RDONLY, &node));
   EXPECT_EQ(0, node->GetStat(&statbuf));
-  EXPECT_EQ(S_IFDIR, statbuf.st_mode & S_IFMT);
-  EXPECT_EQ(0755, statbuf.st_mode & ~S_IFMT);
+  EXPECT_TRUE(S_ISDIR(statbuf.st_mode));
+  EXPECT_EQ(0755, statbuf.st_mode & S_MODEBITS);
 
   // Create a file and stat.
   ASSERT_EQ(0, fs_.Open(Path("/foobar"), O_RDWR | O_CREAT, &node));
   EXPECT_EQ(0, node->GetStat(&statbuf));
-  EXPECT_EQ(S_IFREG, statbuf.st_mode & S_IFMT);
-  EXPECT_EQ(0666, statbuf.st_mode & ~S_IFMT);
+  EXPECT_TRUE(S_ISREG(statbuf.st_mode));
+  EXPECT_EQ(0666, statbuf.st_mode & S_MODEBITS);
   EXPECT_EQ(0, statbuf.st_size);
 }
 
@@ -394,12 +394,12 @@ TEST_F(FuseFsTest, Fchmod) {
 
   ASSERT_EQ(0, fs_.Open(Path("/hello"), O_RDONLY, &node));
   ASSERT_EQ(0, node->GetStat(&statbuf));
-  EXPECT_EQ(0666, statbuf.st_mode & ~S_IFMT);
+  EXPECT_EQ(0666, statbuf.st_mode & S_MODEBITS);
 
   ASSERT_EQ(0, node->Fchmod(0777));
 
   ASSERT_EQ(0, node->GetStat(&statbuf));
-  EXPECT_EQ(0777, statbuf.st_mode & ~S_IFMT);
+  EXPECT_EQ(0777, statbuf.st_mode & S_MODEBITS);
 }
 
 namespace {

native_client_sdk/src/tests/nacl_io_test/html5_fs_test.cc

@@ -184,7 +184,7 @@ TEST_F(Html5FsTest, Mkdir) {
   ScopedNode node;
   ASSERT_EQ(0, fs->Open(path, O_RDONLY, &node));
   EXPECT_EQ(0, node->GetStat(&stat));
-  EXPECT_EQ(S_IFDIR, stat.st_mode & S_IFDIR);
+  EXPECT_TRUE(S_ISDIR(stat.st_mode));
 }
 
 TEST_F(Html5FsTest, Remove) {
@@ -388,8 +388,8 @@ TEST_F(Html5FsTest, GetStat) {
 
   struct stat statbuf;
   EXPECT_EQ(0, node->GetStat(&statbuf));
-  EXPECT_EQ(S_IFREG, statbuf.st_mode & S_IFMT);
-  EXPECT_EQ(S_IRALL | S_IWALL | S_IXALL, statbuf.st_mode & ~S_IFMT);
+  EXPECT_TRUE(S_ISREG(statbuf.st_mode));
+  EXPECT_EQ(S_IRALL | S_IWALL | S_IXALL, statbuf.st_mode & S_MODEBITS);
   EXPECT_EQ(strlen(contents), statbuf.st_size);
   EXPECT_EQ(access_time, statbuf.st_atime);
   EXPECT_EQ(creation_time, statbuf.st_ctime);
@@ -406,8 +406,8 @@ TEST_F(Html5FsTest, GetStat) {
   // GetStat on a directory...
   EXPECT_EQ(0, fs->Open(Path("/dir"), O_RDONLY, &node));
   EXPECT_EQ(0, node->GetStat(&statbuf));
-  EXPECT_EQ(S_IFDIR, statbuf.st_mode & S_IFMT);
-  EXPECT_EQ(S_IRALL | S_IWALL | S_IXALL, statbuf.st_mode & ~S_IFMT);
+  EXPECT_TRUE(S_ISDIR(statbuf.st_mode));
+  EXPECT_EQ(S_IRALL | S_IWALL | S_IXALL, statbuf.st_mode & S_MODEBITS);
   EXPECT_EQ(0, statbuf.st_size);
   EXPECT_EQ(access_time, statbuf.st_atime);
   EXPECT_EQ(creation_time, statbuf.st_ctime);

native_client_sdk/src/tests/nacl_io_test/http_fs_test.cc

@@ -243,7 +243,8 @@ TEST_P(HttpFsLargeFileTest, GetStat) {
 
   struct stat statbuf;
   EXPECT_EQ(0, node->GetStat(&statbuf));
-  EXPECT_EQ(S_IFREG | S_IRUSR | S_IRGRP | S_IROTH, statbuf.st_mode);
+  EXPECT_TRUE(S_ISREG(statbuf.st_mode));
+  EXPECT_EQ(S_IRUSR | S_IRGRP | S_IROTH, statbuf.st_mode & S_MODEBITS);
   EXPECT_EQ(size, statbuf.st_size);
   // These are not currently set.
   EXPECT_EQ(0, statbuf.st_atime);

native_client_sdk/src/tests/nacl_io_test/kernel_proxy_test.cc

@@ -567,7 +567,7 @@ TEST_F(KernelProxyTest, OpenWithMode) {
 
   struct stat buf;
   EXPECT_EQ(0, ki_lstat("/foo", &buf));
-  EXPECT_EQ(0723, buf.st_mode & ~S_IFMT);
+  EXPECT_EQ(0723, buf.st_mode & 0777);
 }
 
 TEST_F(KernelProxyTest, CreateWronlyWithReadOnlyMode) {
@@ -663,10 +663,10 @@ TEST_F(KernelProxyTest, Umask) {
 
   struct stat buf;
   EXPECT_EQ(0, ki_stat("/foo", &buf));
-  EXPECT_EQ(0444, buf.st_mode & ~S_IFMT);
+  EXPECT_EQ(0444, buf.st_mode & 0777);
 
   EXPECT_EQ(0, ki_stat("/dir", &buf));
-  EXPECT_EQ(0555, buf.st_mode & ~S_IFMT);
+  EXPECT_EQ(0555, buf.st_mode & 0777);
 
   EXPECT_EQ(0222, ki_umask(0));
 }

native_client_sdk/src/tests/nacl_io_test/mem_fs_node_test.cc

@@ -137,7 +137,8 @@ TEST(MemFsNodeTest, Fchmod) {
 
   struct stat s;
   ASSERT_EQ(0, file.GetStat(&s));
-  EXPECT_EQ(S_IFREG | S_IRALL | S_IWALL, s.st_mode);
+  EXPECT_TRUE(S_ISREG(s.st_mode));
+  EXPECT_EQ(S_IRALL | S_IWALL, s.st_mode & S_MODEBITS);
 
   // Change to read-only.
   EXPECT_EQ(0, file.Fchmod(S_IRALL));