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.

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

WIP Try harder to handle Ctrl+C #18

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
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 committed May 30, 2018

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
commit ea160a349d8ca045029403160707b6518492053c
127 changes: 125 additions & 2 deletions winsup/cygwin/include/cygwin/exit_process.h
Original file line number Diff line number Diff line change
@@ -232,6 +232,123 @@ inject_remote_thread_into_process(HANDLE process, LPTHREAD_START_ROUTINE address
return res;
}

#define PROCESS_BASIC_INFORMATION_SIZE_32 0x18
#define PEB_OFFSET_32 0x4
#define PARAMS_OFFSET_32 0x10
#define CONSOLE_FLAGS_OFFSET_32 0x14

#define PROCESS_BASIC_INFORMATION_SIZE_64 0x30
#define PEB_OFFSET_64 0x8
#define PARAMS_OFFSET_64 0x20
#define CONSOLE_FLAGS_OFFSET_64 0x18

/**
* Adjust the ConsoleFlags to allow the process to accept Ctrl+C.
*
* The handle must be opened with PROCESS_QUERY_INFORMATION | PROCESS_VM_READ | PROCESS_WRITE.
*
* Returns 1 if the ConsoleFlags were cleared, 0 if they had already been cleared, and -1
* on unspecified error.
*/
static int
adjust_console_flag(HANDLE process)
{
union
{
char chars[PROCESS_BASIC_INFORMATION_SIZE_64];
ULONG32 ulong32;
ULONG64 ulong64;
} u;
int res = 0;
char flags;

#ifdef __LP64__
/* Are we looking at a 32-bit process from a 64-bit one? */
if (!NtQueryInformationProcess (process, ProcessWow64Information, &u.chars, 8, NULL) && u.ulong64)
{
SIZE_T size;
if (!ReadProcessMemory (process, (char *)u.ulong64 + PARAMS_OFFSET_32, u.chars, 4, &size) || size != 4)
return -1;
if (!ReadProcessMemory (process, (char *)(ULONG64)u.ulong32 + CONSOLE_FLAGS_OFFSET_32, &flags, 1, &size) || size != 1)
return -1;
if (flags == 1)
{
res = 1;
flags = 0;
if (!WriteProcessMemory (process, (char *)(ULONG64)u.ulong32 + CONSOLE_FLAGS_OFFSET_32, &flags, 1, &size) || size != 1)
return -1;
}
}

if (!NtQueryInformationProcess (process, ProcessBasicInformation, u.chars, PROCESS_BASIC_INFORMATION_SIZE_64, NULL))
{
SIZE_T size;
if (!ReadProcessMemory (process, (char *)*(ULONG64 *)(u.chars + PEB_OFFSET_64) + PARAMS_OFFSET_64, u.chars, 8, &size) || size != 8)
return -1;
if (!ReadProcessMemory (process, (char *)u.ulong64 + CONSOLE_FLAGS_OFFSET_64, &flags, 1, &size) || size != 1)
return -1;
if (flags == 1)
{
res = 1;
flags = 0;
if (!WriteProcessMemory (process, (char *)u.ulong64 + CONSOLE_FLAGS_OFFSET_64, &flags, 1, &size) || size != 1)
return -1;
}
}
#else
if (!NtQueryInformationProcess(process, ProcessBasicInformation, u.chars, PROCESS_BASIC_INFORMATION_SIZE_32, NULL) && (char *)*(ULONG32 *)(u.chars + PEB_OFFSET_32))
{
SIZE_T size;
if (!ReadProcessMemory (process, (char *)*(ULONG32 *)(u.chars + PEB_OFFSET_32) + PARAMS_OFFSET_32, u.chars, 8, &size) || size != 8)
return -1;
if (!ReadProcessMemory (process, (char *)u.ulong32 + CONSOLE_FLAGS_OFFSET_32, &flags, 1, &size) || size != 1)
return -1;
if (flags == 1)
{
res = 1;
flags = 0;
if (!WriteProcessMemory (process, (char *)u.ulong32 + CONSOLE_FLAGS_OFFSET_32, &flags, 1, &size) || size != 1)
return -1;
}
}

/* So maybe this is a 32-bit process looking at a 64-bit one? */
{
HMODULE ntdll = GetModuleHandleA("ntdll.dll");
static NTSTATUS(NTAPI *NtWow64ReadVirtualMemory64)(HANDLE process, ULONG64 address, PVOID buffer, ULONG64 size, PULONG64 count);
static NTSTATUS(NTAPI *NtWow64WriteVirtualMemory64)(HANDLE process, ULONG64 address, PVOID buffer, ULONG64 size, PULONG64 count);
static NTSTATUS(NTAPI *NtWow64QueryInformationProcess64)(HANDLE process, PROCESSINFOCLASS info_class, PVOID buffer, ULONG size, PULONG count);
static int initialized;
ULONG64 size;

if (!initialized)
{
initialized = 1;
NtWow64ReadVirtualMemory64 = (typeof (NtWow64ReadVirtualMemory64))GetProcAddress (ntdll, "NtWow64ReadVirtualMemory64");
NtWow64WriteVirtualMemory64 = (typeof (NtWow64WriteVirtualMemory64))GetProcAddress (ntdll, "NtWow64WriteVirtualMemory64");
NtWow64QueryInformationProcess64 = (typeof (NtWow64QueryInformationProcess64))GetProcAddress (ntdll, "NtWow64QueryInformationProcess64");
}

if (NtWow64ReadVirtualMemory64 && NtWow64WriteVirtualMemory64 && NtWow64QueryInformationProcess64)
if (!NtWow64QueryInformationProcess64 (process, ProcessBasicInformation, u.chars, PROCESS_BASIC_INFORMATION_SIZE_64, NULL))
{
if (NtWow64ReadVirtualMemory64 (process, *(ULONG64 *)(u.chars + PEB_OFFSET_64) + PARAMS_OFFSET_64, u.chars, 8, &size) || size != 8)
return -1;
if (NtWow64ReadVirtualMemory64(process, u.ulong64 + CONSOLE_FLAGS_OFFSET_64, &flags, 1, &size) || size != 1)
return -1;
}
if (flags == 1)
{
res = 1;
flags = 0;
if (NtWow64WriteVirtualMemory64(process, u.ulong64 + CONSOLE_FLAGS_OFFSET_64, &flags, 1, &size) || size != 1)
return -1;
}
}
#endif
return res;
}

/**
* Terminates the process corresponding to the process ID
*
@@ -253,8 +370,14 @@ exit_one_process(HANDLE process, int exit_code)
if (address &&
!inject_remote_thread_into_process(process, address,
signo == SIGINT ?
CTRL_C_EVENT : CTRL_BREAK_EVENT))
return 0;
CTRL_C_EVENT :
CTRL_BREAK_EVENT)) {
DWORD code;
if (signo == SIGINT && !GetExitCodeProcess (process, &code) ||
code == STILL_ACTIVE && adjust_console_flag(process) > 0 &&
!inject_remote_thread_into_process(process, address, CTRL_C_EVENT))
return 0;
}
/* fall-through */
case SIGTERM:
address = get_exit_process_address_for_process(process);