Make chrome_elf use thunks instead of function pointers.

sandbox/win/src/service_resolver_32.cc

Index: sandbox/win/src/service_resolver_32.cc
diff --git a/sandbox/win/src/service_resolver_32.cc b/sandbox/win/src/service_resolver_32.cc
index 2e69dbc9e7e083b74e8f4db47f7e257ce29acf7f..ed93bc882e3ca4444dbe6ea6a843c7b909b08668 100644
--- a/sandbox/win/src/service_resolver_32.cc
+++ b/sandbox/win/src/service_resolver_32.cc
@@ -179,6 +179,64 @@ 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;