Make chrome_elf use thunks instead of function pointers.

sandbox/win/src/service_resolver_32.cc

 // Copyright (c) 2012 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.
 
 #include "sandbox/win/src/service_resolver.h"
 
 #include "base/memory/scoped_ptr.h"
 #include "sandbox/win/src/win_utils.h"
 
 namespace {
@@ skipping 161 matching lines @@
   if (NULL != storage_used)
     *storage_used = thunk_bytes;
 
   return ret;
 }
 
 size_t ServiceResolverThunk::GetThunkSize() const {
   return offsetof(ServiceFullThunk, internal_thunk) + GetInternalThunkSize();
 }
 
+NTSTATUS ServiceResolverThunk::CopyThunk(const void* target_module,
+                                         const char* target_name,
+                                         void* thunk_storage,
+                                         size_t storage_bytes,
+                                         size_t* storage_used) {
+  NTSTATUS ret = ResolveTarget(target_module, target_name, &target_);
+  if(!NT_SUCCESS(ret))

[robertshield, 2014/02/28 21:02:22]

space after if

[Cait (Slow), 2014/03/03 20:55:11]

Done.

+    return ret;
+
+  size_t thunk_bytes = GetThunkSize();

[rvargas (doing something else), 2014/02/28 19:40:09]

I'm fine with this code as is, but I would also be fine if the contract is to
return only the service code (without the space to store the actual thunk
(InternalThunk), given that there will be no interception). Just saying.

+  scoped_ptr<char[]> thunk_buffer(new char[thunk_bytes]);
+  ServiceFullThunk* thunk = reinterpret_cast<ServiceFullThunk*>(
+                                thunk_buffer.get());
+
+  if (!IsFunctionAService(&thunk->original) &&
+      (!relaxed_ || !SaveOriginalFunction(&thunk->original, thunk_storage))) {
+    return STATUS_UNSUCCESSFUL;
+ }
+
+  BYTE* thunk_storage_bytes = reinterpret_cast<BYTE*>(thunk_storage);

[rvargas (doing something else), 2014/02/28 19:40:09]

Why have the rest of this code here? We can assume that there's no need to copy
this to the child (no?). As for the memory in this process, the caller should
collect all the services into a single buffer and protect the whole thing at
once.

[robertshield, 2014/02/28 21:02:22]

I agree with this, I think the memory protection code can be moved out to the
caller.

[Cait (Slow), 2014/03/03 20:55:11]

Done.

+
+  // Mark the thunk storage as readable and writeable, since we
+  // ready to write to it.
+  DWORD old_protect = 0;
+  if(!VirtualProtect(thunk_storage,

[robertshield, 2014/02/28 21:02:22]

space after if

[Cait (Slow), 2014/03/03 20:55:11]

Done.

+                     sizeof(thunk_storage),
+                     PAGE_EXECUTE_READWRITE,
+                     &old_protect)) {
+    return STATUS_UNSUCCESSFUL;
+  }
+
+  // copy the local thunk buffer to the child
+  SIZE_T written;

[robertshield, 2014/02/28 21:02:22]

= 0

[Cait (Slow), 2014/03/03 20:55:11]

Done.

+  ret = ::WriteProcessMemory(process_,
+                             thunk_storage_bytes,
+                             reinterpret_cast<void*>(&thunk->original),
+                             thunk_bytes,
+                             &written);
+
+  if (!NT_SUCCESS(ret))
+    return ret;
+
+  if (thunk_bytes != written)
+    return STATUS_UNSUCCESSFUL;
+
+  if (! VirtualProtect(thunk_storage,

[robertshield, 2014/02/28 21:02:22]

nit: no space after !

[Cait (Slow), 2014/03/03 20:55:11]

Done.

+                       sizeof(thunk_storage),
+                       PAGE_EXECUTE_READ,
+                       &old_protect)) {
+    return STATUS_UNSUCCESSFUL;
+  }
+
+  if (NULL != storage_used)
+    *storage_used = thunk_bytes;
+
+  return ret;
+}
+
 bool ServiceResolverThunk::IsFunctionAService(void* local_thunk) const {
   ServiceEntry function_code;
   SIZE_T read;
   if (!::ReadProcessMemory(process_, target_, &function_code,
                            sizeof(function_code), &read))
     return false;
 
   if (sizeof(function_code) != read)
     return false;
 
@@ skipping 219 matching lines @@
     return false;
   }
 
   // Save the verified code
   memcpy(local_thunk, &function_code, sizeof(function_code));
 
   return true;
 }
 
 }  // namespace sandbox