[Sandbox service resolver hooks] Remove the RET hijacking in ntdll.

Changed the x64 ntdll hook to use JMP instead of RET.
(Also a quick fix to a DCHECK_NT with wrong logic.)

See bug ticket for x86/wow64 details (no changes needed).

BUG=510170
R=jschuh@chromium.org,wfh@chromium.org

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

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

2 files changed, 9 insertions, 18 deletions

diff --git a/sandbox/win/src/resolver_64.cc b/sandbox/win/src/resolver_64.cc
index 8b2cc53..f1f135e 100644
--- a/sandbox/win/src/resolver_64.cc
+++ b/sandbox/win/src/resolver_64.cc
@@ -12,34 +12,25 @@
 
 namespace {
 
-const BYTE kPushRax = 0x50;
 const USHORT kMovRax = 0xB848;
-const ULONG kMovRspRax = 0x24048948;
-const BYTE kRetNp = 0xC3;
+const USHORT kJmpRax = 0xe0ff;
 
 #pragma pack(push, 1)
 struct InternalThunk {
   // This struct contains roughly the following code:
-  // 00 50                    push  rax
   // 01 48b8f0debc9a78563412  mov   rax,123456789ABCDEF0h
-  // 0b 48890424              mov   qword ptr [rsp],rax
-  // 0f c3                    ret
+  // ff e0                    jmp   rax
   //
-  // The code modifies rax, but that should not be an issue for the common
-  // calling conventions.
+  // The code modifies rax, but that's fine for x64 ABI.
 
   InternalThunk() {
-    push_rax = kPushRax;
     mov_rax = kMovRax;
+    jmp_rax = kJmpRax;
     interceptor_function = 0;
-    mov_rsp_rax = kMovRspRax;
-    ret = kRetNp;
   };
-  BYTE push_rax;        // = 50
   USHORT mov_rax;       // = 48 B8
   ULONG_PTR interceptor_function;
-  ULONG mov_rsp_rax;    // = 48 89 04 24
-  BYTE ret;             // = C3
+  USHORT jmp_rax;  // = ff e0
 };
 #pragma pack(pop)
 
diff --git a/sandbox/win/src/service_resolver_64.cc b/sandbox/win/src/service_resolver_64.cc
index c0e684c..8dcea7d 100644
--- a/sandbox/win/src/service_resolver_64.cc
+++ b/sandbox/win/src/service_resolver_64.cc
@@ -139,9 +139,9 @@ NTSTATUS ServiceResolverThunk::Setup(const void* target_module,
                                      void* thunk_storage,
                                      size_t storage_bytes,
                                      size_t* storage_used) {
-  NTSTATUS ret = Init(target_module, interceptor_module, target_name,
-                      interceptor_name, interceptor_entry_point,
-                      thunk_storage, storage_bytes);
+  NTSTATUS ret =
+      Init(target_module, interceptor_module, target_name, interceptor_name,
+           interceptor_entry_point, thunk_storage, storage_bytes);
   if (!NT_SUCCESS(ret))
     return ret;
 
@@ -213,7 +213,7 @@ NTSTATUS ServiceResolverThunk::PerformPatch(void* local_thunk,
                                             void* remote_thunk) {
   // Patch the original code.
   ServiceEntry local_service;
-  DCHECK_NT(GetInternalThunkSize() >= sizeof(local_service));
+  DCHECK_NT(GetInternalThunkSize() <= sizeof(local_service));
   if (!SetInternalThunk(&local_service, sizeof(local_service), NULL,
                         interceptor_))
     return STATUS_UNSUCCESSFUL;