Do not translate the file open flags other than read/write.

After r200088, NACL_ABI_O_CREAT, NACL_ABI_O_TRUNC, and NACL_ABI_O_EXCL are passed to NaClDescIoDescFromHandleAllocCtor.
But the function does not expect these flags and that causes NaCl to abort.
This CL changes NaClIPCAdapter so that is does not translate the file open flags other than PP_FILEOPENFLAG_READ and PP_FILEOPENFLAG_WRITE.

BUG=241726
TEST=unit_tests --gtest_filter="NaClIPCAdapterTest.*"

Review URL: https://chromiumcodereview.appspot.com/15274002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@202027 0039d316-1c4b-4281-b951-d872f2087c98

3 files changed, 62 insertions, 24 deletions

diff --git a/chrome/nacl/nacl_ipc_adapter.cc b/chrome/nacl/nacl_ipc_adapter.cc
index 58c6fb2..45222ae 100644
--- a/chrome/nacl/nacl_ipc_adapter.cc
+++ b/chrome/nacl/nacl_ipc_adapter.cc
@@ -96,31 +96,25 @@ void DeleteChannel(IPC::Channel* channel) {
   delete channel;
 }
 
-int TranslatePepperFileOpenFlags(int32_t pp_open_flags) {
+// Translates Pepper's read/write open flags into NaCl's ones. The other open
+// flags are discarded. If neither of the read/write flags is specified, just
+// returns NACL_ABI_O_RDONLY as a safe fallback.
+int TranslatePepperFileReadWriteOpenFlags(int32_t pp_open_flags) {
   int nacl_open_flag;
-  switch (pp_open_flags & (PP_FILEOPENFLAG_READ | PP_FILEOPENFLAG_WRITE)) {
-    case PP_FILEOPENFLAG_READ:
-      nacl_open_flag = NACL_ABI_O_RDONLY;
-      break;
-    case PP_FILEOPENFLAG_WRITE:
-      nacl_open_flag = NACL_ABI_O_WRONLY;
-      break;
-    case PP_FILEOPENFLAG_READ | PP_FILEOPENFLAG_WRITE:
-      nacl_open_flag = NACL_ABI_O_RDWR;
-      break;
-    default:
-      // NACL_ABI_O_RDONLY == 0, so make this ambiguous case readonly as a safe
-      // fallback.
-      nacl_open_flag = NACL_ABI_O_RDONLY;
-      break;
+  if ((pp_open_flags & (PP_FILEOPENFLAG_READ | PP_FILEOPENFLAG_WRITE)) ==
+      (PP_FILEOPENFLAG_READ | PP_FILEOPENFLAG_WRITE)) {
+    nacl_open_flag = NACL_ABI_O_RDWR;
+  } else if (pp_open_flags & PP_FILEOPENFLAG_READ) {
+    nacl_open_flag = NACL_ABI_O_RDONLY;
+  } else if (pp_open_flags & PP_FILEOPENFLAG_WRITE) {
+    nacl_open_flag = NACL_ABI_O_WRONLY;
+  } else {
+    DLOG(WARNING) << "PP_FILEOPENFLAG_READ and/or PP_FILEOPENFLAG_WRITE "
+                  << "should be specified.";
+    // NACL_ABI_O_RDONLY == 0, so make this ambiguous case readonly as a safe
+    // fallback.
+    nacl_open_flag = NACL_ABI_O_RDONLY;
   }
-
-  if (pp_open_flags & PP_FILEOPENFLAG_CREATE)
-    nacl_open_flag |= NACL_ABI_O_CREAT;
-  if (pp_open_flags & PP_FILEOPENFLAG_TRUNCATE)
-    nacl_open_flag |= NACL_ABI_O_TRUNC;
-  if (pp_open_flags & PP_FILEOPENFLAG_EXCLUSIVE)
-    nacl_open_flag |= NACL_ABI_O_EXCL;
   return nacl_open_flag;
 }
 
@@ -445,7 +439,7 @@ bool NaClIPCAdapter::OnMessageReceived(const IPC::Message& msg) {
 #else
               iter->descriptor().fd,
 #endif
-              TranslatePepperFileOpenFlags(iter->open_flag()))));
+              TranslatePepperFileReadWriteOpenFlags(iter->open_flag()))));
           break;
         case ppapi::proxy::SerializedHandle::INVALID: {
           // Nothing to do. TODO(dmichael): Should we log this? Or is it
@@ -584,3 +578,7 @@ void NaClIPCAdapter::SaveMessage(const IPC::Message& msg,
   rewritten_msg->SetData(header, msg.payload(), msg.payload_size());
   locked_data_.to_be_received_.push(rewritten_msg);
 }
+
+int TranslatePepperFileReadWriteOpenFlagsForTesting(int32_t pp_open_flags) {
+  return TranslatePepperFileReadWriteOpenFlags(pp_open_flags);
+}
diff --git a/chrome/nacl/nacl_ipc_adapter.h b/chrome/nacl/nacl_ipc_adapter.h
index 515b0fc..c7569e6 100644
--- a/chrome/nacl/nacl_ipc_adapter.h
+++ b/chrome/nacl/nacl_ipc_adapter.h
@@ -20,6 +20,7 @@
 #include "base/synchronization/lock.h"
 #include "base/task_runner.h"
 #include "ipc/ipc_listener.h"
+#include "ppapi/c/pp_stdint.h"
 #include "ppapi/proxy/handle_converter.h"
 
 struct NaClDesc;
@@ -185,4 +186,7 @@ class NaClIPCAdapter : public base::RefCountedThreadSafe<NaClIPCAdapter>,
   DISALLOW_COPY_AND_ASSIGN(NaClIPCAdapter);
 };
 
+// Export TranslatePepperFileReadWriteOpenFlags for testing.
+int TranslatePepperFileReadWriteOpenFlagsForTesting(int32_t pp_open_flags);
+
 #endif  // CHROME_NACL_NACL_IPC_ADAPTER_H_
diff --git a/chrome/nacl/nacl_ipc_adapter_unittest.cc b/chrome/nacl/nacl_ipc_adapter_unittest.cc
index 8a334d5..9258bd1 100644
--- a/chrome/nacl/nacl_ipc_adapter_unittest.cc
+++ b/chrome/nacl/nacl_ipc_adapter_unittest.cc
@@ -13,6 +13,8 @@
 #include "base/threading/simple_thread.h"
 #include "ipc/ipc_test_sink.h"
 #include "native_client/src/trusted/desc/nacl_desc_custom.h"
+#include "native_client/src/trusted/service_runtime/include/sys/fcntl.h"
+#include "ppapi/c/ppb_file_io.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace {
@@ -306,3 +308,37 @@ TEST_F(NaClIPCAdapterTest, ReadWithChannelError) {
   thread.Join();
 }
 
+// Tests that TranslatePepperFileOpenFlags translates pepper read/write open
+// flags into NaCl open flags correctly.
+TEST_F(NaClIPCAdapterTest, TranslatePepperFileReadWriteOpenFlags) {
+  EXPECT_EQ(NACL_ABI_O_RDONLY,
+      TranslatePepperFileReadWriteOpenFlagsForTesting(PP_FILEOPENFLAG_READ));
+  EXPECT_EQ(NACL_ABI_O_WRONLY,
+      TranslatePepperFileReadWriteOpenFlagsForTesting(PP_FILEOPENFLAG_WRITE));
+  EXPECT_EQ(NACL_ABI_O_RDWR,
+      TranslatePepperFileReadWriteOpenFlagsForTesting(
+          PP_FILEOPENFLAG_READ | PP_FILEOPENFLAG_WRITE));
+
+  // The flags other than PP_FILEOPENFLAG_READ and PP_FILEOPENFLAG_WRITE are
+  // discared.
+  EXPECT_EQ(NACL_ABI_O_WRONLY,
+      TranslatePepperFileReadWriteOpenFlagsForTesting(
+          PP_FILEOPENFLAG_WRITE | PP_FILEOPENFLAG_CREATE));
+  EXPECT_EQ(NACL_ABI_O_WRONLY,
+      TranslatePepperFileReadWriteOpenFlagsForTesting(
+          PP_FILEOPENFLAG_WRITE | PP_FILEOPENFLAG_TRUNCATE));
+  EXPECT_EQ(NACL_ABI_O_WRONLY,
+      TranslatePepperFileReadWriteOpenFlagsForTesting(
+          PP_FILEOPENFLAG_WRITE | PP_FILEOPENFLAG_EXCLUSIVE));
+
+  // If neither of PP_FILEOPENFLAG_READ and PP_FILEOPENFLAG_WRITE is set, falls
+  // back NACL_ABI_O_READONLY
+  EXPECT_EQ(NACL_ABI_O_RDONLY,
+      TranslatePepperFileReadWriteOpenFlagsForTesting(PP_FILEOPENFLAG_CREATE));
+  EXPECT_EQ(NACL_ABI_O_RDONLY,
+      TranslatePepperFileReadWriteOpenFlagsForTesting(
+          PP_FILEOPENFLAG_TRUNCATE));
+  EXPECT_EQ(NACL_ABI_O_RDONLY,
+      TranslatePepperFileReadWriteOpenFlagsForTesting(
+          PP_FILEOPENFLAG_EXCLUSIVE));
+}