Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

WIP Try harder to handle Ctrl+C #18

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

WIP Try harder to handle Ctrl+C #18

wants to merge 2 commits into from

Commits on May 30, 2018

  1. Try harder to deliver SIGINT 

    It seems that the "signal" associated with a Ctrl+C (i.e. a
CTRL_C_EVENT) is sometimes not passed through to the registered handler.

This symptom occurs e.g. when spawning a non-MSYS2 process in Bash
through env.exe, such as is commonly the case when starting ruby scripts
via the shebang line `#!/usr/bin/env ruby` (which usually calls
/mingw64/bin/ruby.exe). The unexpected behavior is that a CtrlRoutine
thread can be executed successfully but does not result in the process'
handler to be called, let alone in the process being terminated.

Even more curious is that a CTRL_BREAK_EVENT is delivered without
problems.

A rather intense few days of quality time with GDB and DuckDuckGo turned
into the following analysis of the issue:

It turns out that after calling EnterCriticalSection(), CtrlRoutine()
asks whether it has been passed 0 (CTRL_C_EVENT) as a parameter and in
that case, jumps to conditional code (hex addresses removed from GDB's
disassembly to save on space):

<KERNELBASE!CtrlRoutine+91>:  callq  *0x17c9af(%rip)
<KERNELBASE!CtrlRoutine+97>:  andl   $0x0,0x24(%rsp)
<KERNELBASE!CtrlRoutine+102>: test   %ebx,%ebx
<KERNELBASE!CtrlRoutine+104>: je     <KERNELBASE!CtrlRoutine+163>

That conditional code reads thusly:

<KERNELBASE!CtrlRoutine+163>: mov    %gs:0x60,%rax
<KERNELBASE!CtrlRoutine+172>: mov    0x20(%rax),%rcx
<KERNELBASE!CtrlRoutine+176>: testb  $0x1,0x18(%rcx)
<KERNELBASE!CtrlRoutine+180>: jne    <KERNELBASE!CtrlRoutine+216>
<KERNELBASE!CtrlRoutine+182>: jmp    <KERNELBASE!CtrlRoutine+106>

This code looks at %gs:0x60, which according to
https://en.wikipedia.org/wiki/Win32_Thread_Information_Block is the PEB
(Process Environment Block). Then it accesses the field of the PEB at
offset 0x20, which is the ProcessParameters field according to
https://msdn.microsoft.com/en-us/library/windows/desktop/aa813706.aspx

These process parameters are described here:
http://undocumented.ntinternals.net/UserMode/Structures/RTL_USER_PROCESS_PARAMETERS.html

Sadly, it is unclear what the field at offset 0x18 (named
`ConsoleFlags`) does, but from the disassembly it is clear that if it
has its least significant bit set, CtrlRoutine() simply cleans up and
exits. The handler(s) only get a chance to run when that bit is 0. (Note
that ReactOS expects *all* of ConsoleFlags to be different from 1:
https://github.com/reactos/reactos/blob/ae9702fce/dll/win32/kernel32/client/console/console.c#L169)

Note: When a 64-bit process asks for the PEB of a 32-bit process, it
does receive a copy of *a 64-bit version* of it. This 64-bit version
points to a copy of the USER_PROCESS_INFORMATION struct that is
compatible with 64-bit, but writing to it will not have any effect! We
instead need to access the 32-bit PEB, which has a pointer to the 32-bit
version of the process information, where we modify the ConsoleFlags. We
are using a special trick to obtain the 32-bit PEB inspired by
https://stackoverflow.com/a/23842609: asking NtQueryInformation() for
the ProcessWow64Information will return the 64-bit address of the 32-bit
PEB, if there is any (indicating that the process in question is a WoW
processes i.e. 32-bit processes running on 64-bit Windows).

This closes git-for-windows/git#1648 and
git-for-windows/git#1470

Signed-off-by: Johannes Schindelin <[email protected]>
    @dscho
    dscho committed May 30, 2018
    Configuration menu
    Copy the full SHA
    ea160a3 View commit details
    Browse the repository at this point in the history

  2. TODO: use IsProcessCritical() when available 

    Also TODO: test on 32-bit

Also TODO: identify more tickets on GitHub that would probably be fixed
by this topic branch

Signed-off-by: Johannes Schindelin <[email protected]>
    @dscho
    dscho committed May 30, 2018
    Configuration menu
    Copy the full SHA
    e91f623 View commit details
    Browse the repository at this point in the history