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

[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

Committed: https://crrev.com/5c361825548ff9720c0e3544c685e951c5b5c0fd
Cr-Commit-Position: refs/heads/master@{#364220}

[jschuh, 2015-12-08 01:19:42]

Lgtm, but I should warn you that you may want to keep an eye on stability
numbers, on the off chance that e.g. some AV is relying on our hook format.

https://codereview.chromium.org/1504943002/diff/1/sandbox/win/src/service_res...
File sandbox/win/src/service_resolver_64.cc (right):

https://codereview.chromium.org/1504943002/diff/1/sandbox/win/src/service_res...
sandbox/win/src/service_resolver_64.cc:216: DCHECK_NT(GetInternalThunkSize() <=
sizeof(local_service));
Heh, that's an interesting one.

[penny, 2015-12-09 21:56:13]

On 2015/12/08 01:19:42, jschuh (very slow) wrote:
> Lgtm, but I should warn you that you may want to keep an eye on stability
> numbers, on the off chance that e.g. some AV is relying on our hook format.
> 
>
https://codereview.chromium.org/1504943002/diff/1/sandbox/win/src/service_res...
> File sandbox/win/src/service_resolver_64.cc (right):
> 
>
https://codereview.chromium.org/1504943002/diff/1/sandbox/win/src/service_res...
> sandbox/win/src/service_resolver_64.cc:216: DCHECK_NT(GetInternalThunkSize()
<=
> sizeof(local_service));
> Heh, that's an interesting one.

Good point, well made.  Will keep an eye on stability numbers, and listen for
screaming users.

The DCHECK_NT was firing all the time while I was trying to windbg, so fixed
that quick.  It takes a sec to wrap your head around the logic (DCHECK_NT
asserts if FALSE).

Thanks for the review!

[penny, 2015-12-09 21:58:15]

The CQ bit was checked by pennymac@chromium.org

[commit-bot: I haz the power, 2015-12-09 21:59:58]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1504943002/1
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1504943002/1

[commit-bot: I haz the power, 2015-12-09 23:44:15]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-12-09 23:44:59]

Description was changed from

==========
[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
==========

to

==========
[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

Committed: https://crrev.com/5c361825548ff9720c0e3544c685e951c5b5c0fd
Cr-Commit-Position: refs/heads/master@{#364220}
==========

[commit-bot: I haz the power, 2015-12-09 23:45:00]

Patchset 1 (id:??) landed as
https://crrev.com/5c361825548ff9720c0e3544c685e951c5b5c0fd
Cr-Commit-Position: refs/heads/master@{#364220}