Porting sbox IPC to 64 bit

-IPC unit tests enabled
-Integration IPCTest test enabled
-All unit test pass (62 tests)

Other tests require interceptor fu.


BUG=27218
TEST= unit tests included


Review URL: http://codereview.chromium.org/661299

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

9 files changed, 65 insertions, 49 deletions

diff --git a/sandbox/sandbox.gyp b/sandbox/sandbox.gyp
index 5cc70ee..b704be7 100644
--- a/sandbox/sandbox.gyp
+++ b/sandbox/sandbox.gyp
@@ -78,6 +78,10 @@
             'src/service_resolver.h',
             'src/shared_handles.cc',
             'src/shared_handles.h',
+            'src/sharedmem_ipc_client.cc',
+            'src/sharedmem_ipc_client.h',
+            'src/sharedmem_ipc_server.cc',
+            'src/sharedmem_ipc_server.h',            
             'src/sid.cc',
             'src/sid.h',
             'src/sync_policy.cc',
@@ -211,10 +215,6 @@
             'src/registry_interception.h',
             'src/resolver_32.cc',
             'src/service_resolver_32.cc',
-            'src/sharedmem_ipc_client.cc',
-            'src/sharedmem_ipc_client.h',
-            'src/sharedmem_ipc_server.cc',
-            'src/sharedmem_ipc_server.h',
             'src/sidestep_resolver.cc',
             'src/sidestep_resolver.h',
             'src/sidestep\ia32_modrm_map.cpp',
diff --git a/sandbox/src/internal_types.h b/sandbox/src/internal_types.h
index 8c72262..4834975 100644
--- a/sandbox/src/internal_types.h
+++ b/sandbox/src/internal_types.h
@@ -1,9 +1,9 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2006-2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#ifndef SANDBOX_SRC_INTERNAL_TYPES_H__
-#define SANDBOX_SRC_INTERNAL_TYPES_H__
+#ifndef SANDBOX_SRC_INTERNAL_TYPES_H_
+#define SANDBOX_SRC_INTERNAL_TYPES_H_
 
 namespace sandbox {
 
@@ -42,6 +42,34 @@ class CountedBuffer {
   void* buffer_;
 };
 
+// Helper class to convert void-pointer packed ints for both
+// 32 and 64 bit builds. This construct is non-portable.
+class IPCInt {
+ public:
+  explicit IPCInt(void* buffer) {
+    buffer_.vp = buffer;
+  }
+
+  explicit IPCInt(unsigned __int32 i32) {
+    buffer_.vp = NULL;
+    buffer_.i32 = i32;
+  }
+
+  unsigned __int32 As32Bit() const {
+    return buffer_.i32;
+  }
+
+  void* AsVoidPtr() const {
+    return buffer_.vp;
+  }
+
+ private:
+  union U {
+    void* vp;
+    unsigned __int32 i32;
+  } buffer_;
+};
+
 }  // namespace sandbox
 
-#endif  // SANDBOX_SRC_INTERNAL_TYPES_H__
+#endif  // SANDBOX_SRC_INTERNAL_TYPES_H_
diff --git a/sandbox/src/ipc_unittest.cc b/sandbox/src/ipc_unittest.cc
index cb9e81c..1ba24ab 100644
--- a/sandbox/src/ipc_unittest.cc
+++ b/sandbox/src/ipc_unittest.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2006-2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -40,10 +40,15 @@ TEST(IPCTest, ChannelMaker) {
   IPCControl* client_control = MakeChannels(12*64, 4096, &channel_start);
 
   // Test that our testing rig is computing offsets properly. We should have
-  // 5 channnels and the offset to the first channel is 108 bytes.
+  // 5 channnels and the offset to the first channel is 108 bytes in 32 bits
+  // and 216 in 64 bits.
   ASSERT_TRUE(NULL != client_control);
   EXPECT_EQ(5, client_control->channels_count);
+#if defined(_WIN64)
+  EXPECT_EQ(216, channel_start);
+#else
   EXPECT_EQ(108, channel_start);
+#endif
 
   delete [] reinterpret_cast<char*>(client_control);
 }
@@ -219,7 +224,7 @@ TEST(IPCTest, CrossCallValidation) {
   // First a sanity test with a well formed parameter object.
   unsigned long value = 124816;
   const uint32 kTag = 33;
-  ActualCallParams<1, 128> params_1(kTag);
+  ActualCallParams<1, 256> params_1(kTag);
   params_1.CopyParamIn(0, &value, sizeof(value), false, ULONG_TYPE);
   void* buffer = const_cast<void*>(params_1.GetBuffer());
 
@@ -240,7 +245,7 @@ TEST(IPCTest, CrossCallValidation) {
   const int32 kPtrDiffSz = sizeof(ptrdiff_t);
   for (int32 ix = -1; ix != 3; ++ix) {
     uint32 fake_num_params = (kuint32max / kPtrDiffSz) + ix;
-    ActualCallParams<1, 128> params_2(kTag, fake_num_params);
+    ActualCallParams<1, 256> params_2(kTag, fake_num_params);
     params_2.CopyParamIn(0, &value, sizeof(value), false, ULONG_TYPE);
     buffer = const_cast<void*>(params_2.GetBuffer());
 
diff --git a/sandbox/src/policy_broker.cc b/sandbox/src/policy_broker.cc
index 1b26938..82701ed 100644
--- a/sandbox/src/policy_broker.cc
+++ b/sandbox/src/policy_broker.cc
@@ -89,7 +89,7 @@ bool SetupNtdllImports(TargetProcess *child) {
 
 bool SetupBasicInterceptions(InterceptionManager* manager) {
 #if !defined(_WIN64)
-  // Bug 27218: We don't have IPC yet.
+  // Bug 27218: We don't have dispatch for some x64 syscalls.
   // Interceptions provided by process_thread_policy, without actual policy.
   if (!INTERCEPT_NT(manager, NtOpenThread, OPEN_TREAD_ID, 20) ||
       !INTERCEPT_NT(manager, NtOpenProcess, OPEN_PROCESS_ID, 20) ||
@@ -105,7 +105,7 @@ bool SetupBasicInterceptions(InterceptionManager* manager) {
 
   if (win_util::GetWinVersion() >= win_util::WINVERSION_XP) {
 #if !defined(_WIN64)
-    // Bug 27218: We don't have IPC yet.
+    // Bug 27218: We don't have dispatch for some x64 syscalls.
     // This one is also provided by process_thread_policy.
     if (!INTERCEPT_NT(manager, NtOpenProcessTokenEx, OPEN_PROCESS_TOKEN_EX_ID,
                       20))
diff --git a/sandbox/src/policy_target_test.cc b/sandbox/src/policy_target_test.cc
index 042d455..6a7aebb 100644
--- a/sandbox/src/policy_target_test.cc
+++ b/sandbox/src/policy_target_test.cc
@@ -111,8 +111,9 @@ SBOX_TESTS_COMMAND int PolicyTargetTest_thread(int argc, wchar_t **argv) {
   HANDLE thread = ::OpenThread(SYNCHRONIZE, FALSE, thread_id);
   if (!thread)
     return ::GetLastError();
+  if (!::CloseHandle(thread))
+    return ::GetLastError();
 
-  ::CloseHandle(thread);
   return SBOX_TEST_SUCCEEDED;
 }
 
@@ -130,12 +131,15 @@ SBOX_TESTS_COMMAND int PolicyTargetTest_thread2(int argc, wchar_t **argv) {
                                  &thread_id);
   if (!thread)
     return ::GetLastError();
-  ::CloseHandle(thread);
+  if (!::CloseHandle(thread))
+    return ::GetLastError();
 
   thread = ::OpenThread(SYNCHRONIZE, FALSE, thread_id);
   if (!thread)
     return ::GetLastError();
-  ::CloseHandle(thread);
+
+  if (!::CloseHandle(thread))
+    return ::GetLastError();
 
   return SBOX_TEST_SUCCEEDED;
 }
@@ -191,7 +195,7 @@ TEST(PolicyTargetTest, OpenThreadTokenEx) {
 }
 
 #if !defined(_WIN64)
-// Bug 27218: We don't have IPC yet.
+// Bug 27218: We don't have dispatch for some x64 syscalls.
 TEST(PolicyTargetTest, OpenThread) {
   TestRunner runner;
   EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"PolicyTargetTest_thread")) <<
diff --git a/sandbox/src/sandbox_policy_base.cc b/sandbox/src/sandbox_policy_base.cc
index 868c841..d47d5921 100644
--- a/sandbox/src/sandbox_policy_base.cc
+++ b/sandbox/src/sandbox_policy_base.cc
@@ -72,7 +72,7 @@ PolicyBase::PolicyBase()
   memset(&ipc_targets_, NULL, sizeof(ipc_targets_));
   Dispatcher* dispatcher = NULL;
 #if !defined(_WIN64)
-  // Bug 27218: We don't have IPC yet.
+  // Bug 27218: We don't have dispatch for some x64 syscalls.
   dispatcher = new FilesystemDispatcher(this);
   ipc_targets_[IPC_NTCREATEFILE_TAG] = dispatcher;
   ipc_targets_[IPC_NTOPENFILE_TAG] = dispatcher;
@@ -103,7 +103,7 @@ PolicyBase::~PolicyBase() {
     delete target;
   }
 #if !defined(_WIN64)
-  // Bug 27218: We don't have IPC yet.
+  // Bug 27218: We don't have dispatch for some x64 syscalls.
   delete ipc_targets_[IPC_NTCREATEFILE_TAG];
   delete ipc_targets_[IPC_NTOPENTHREAD_TAG];
   delete ipc_targets_[IPC_CREATENAMEDPIPEW_TAG];
@@ -111,8 +111,8 @@ PolicyBase::~PolicyBase() {
   delete ipc_targets_[IPC_NTCREATEKEY_TAG];
   delete policy_maker_;
   delete policy_;
-  ::DeleteCriticalSection(&lock_);
 #endif
+  ::DeleteCriticalSection(&lock_);
 }
 
 DWORD PolicyBase::MakeJobObject(HANDLE* job) {
@@ -393,21 +393,15 @@ bool PolicyBase::SetupService(InterceptionManager* manager, int service) {
 // IPC subsystem. We receive a integer cookie and we are expected to return the
 // cookie times two (or three) and the current tick count.
 bool PolicyBase::Ping(IPCInfo* ipc, void* arg1) {
-  uint32 tag = ipc->ipc_tag;
-
-  switch (tag) {
-#ifndef _WIN64  // TODO(gregoryd): To build this code for 64-bits Windows we
-                // need to make sure IPC is fully ported to Win64.
+  switch (ipc->ipc_tag) {
     case IPC_PING1_TAG: {
-      uint32 cookie = bit_cast<uint32>(arg1);
-      COMPILE_ASSERT(sizeof(cookie) == sizeof(arg1), breaks_with_64_bit);
-
+      IPCInt ipc_int(arg1);
+      uint32 cookie = ipc_int.As32Bit();
       ipc->return_info.extended_count = 2;
       ipc->return_info.extended[0].unsigned_int = ::GetTickCount();
       ipc->return_info.extended[1].unsigned_int = 2 * cookie;
       return true;
     }
-#endif
     case IPC_PING2_TAG: {
       CountedBuffer* io_buffer = reinterpret_cast<CountedBuffer*>(arg1);
       if (sizeof(uint32) != io_buffer->Size())
diff --git a/sandbox/src/sharedmem_ipc_server.cc b/sandbox/src/sharedmem_ipc_server.cc
index 9846b6c..d835033 100644
--- a/sandbox/src/sharedmem_ipc_server.cc
+++ b/sandbox/src/sharedmem_ipc_server.cc
@@ -1,4 +1,4 @@
-// Copyright (c) 2009 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -176,7 +176,8 @@ bool GetArgs(CrossCallParamsEx* params, IPCParams* ipc_params,
             ReleaseArgs(ipc_params, args);
             return false;
           }
-          args[i] = bit_cast<void*>(data);
+          IPCInt ipc_int(data);
+          args[i] = ipc_int.AsVoidPtr();
           break;
         }
         case INOUTPTR_TYPE: {
diff --git a/sandbox/src/target_process.cc b/sandbox/src/target_process.cc
index fd2d408..a9848c5 100644
--- a/sandbox/src/target_process.cc
+++ b/sandbox/src/target_process.cc
@@ -105,10 +105,7 @@ TargetProcess::~TargetProcess() {
     return;
   }
 
-#if !defined(_WIN64)
-  // Bug 27218: We don't have IPC yet.
   delete ipc_server_;
-#endif
 
   ::CloseHandle(lockdown_token_);
   ::CloseHandle(initial_token_);
@@ -302,14 +299,11 @@ DWORD TargetProcess::Init(Dispatcher* ipc_dispatcher, void* policy,
            ::GetLastError() : ERROR_INVALID_FUNCTION;
   }
 
-#if !defined(_WIN64)
-  // Bug 27218: We don't have IPC yet.
   ipc_server_ = new SharedMemIPCServer(sandbox_process_, sandbox_process_id_,
                                        job_, thread_pool_, ipc_dispatcher);
 
   if (!ipc_server_->Init(shared_memory, shared_IPC_size, kIPCChannelSize))
     return ERROR_NOT_ENOUGH_MEMORY;
-#endif
 
   // After this point we cannot use this handle anymore.
   sandbox_thread_ = NULL;
diff --git a/sandbox/src/target_services.cc b/sandbox/src/target_services.cc
index 2bfe67d..72f6d4c 100644
--- a/sandbox/src/target_services.cc
+++ b/sandbox/src/target_services.cc
@@ -84,29 +84,22 @@ bool TargetServicesBase::TestIPCPing(int version) {
   if (NULL == memory) {
     return false;
   }
-
-#if defined(_WIN64)
-  // Bug 27218: We don't have IPC yet.
-  return false;
-#else
   SharedMemIPCClient ipc(memory);
   CrossCallReturn answer = {0};
 
   if (1 == version) {
     uint32 tick1 = ::GetTickCount();
-    uint32 cookie = 717111;
+    uint32 cookie = 717115;
     ResultCode code = CrossCall(ipc, IPC_PING1_TAG, cookie, &answer);
 
     if (SBOX_ALL_OK != code) {
       return false;
     }
-
     // We should get two extended returns values from the IPC, one is the
     // tick count on the broker and the other is the cookie times two.
     if ((answer.extended_count != 2)) {
       return false;
     }
-
     // We test the first extended answer to be within the bounds of the tick
     // count only if there was no tick count wraparound.
     uint32 tick2 = ::GetTickCount();
@@ -128,16 +121,13 @@ bool TargetServicesBase::TestIPCPing(int version) {
     if (SBOX_ALL_OK != code) {
       return false;
     }
-
     if (cookie != 717111 * 3) {
       return false;
     }
   } else {
     return false;
   }
-
   return true;
-#endif
 }
 
 bool ProcessState::IsKernel32Loaded() {