Fix fork() regression on Windows

Recent optimizations to fork() introduced a regression, that could cause
the subprocess to fail unexpectedly, when TlsAlloc() returns a different
index. This is because we were burning the indexes into the displacement
of x86 opcodes. So when fork() happened and the executable memory copied
it would use the old index. Right now the way this is being solved is to
not copy the executable on fork() and then re-apply code changes. If you
need to be able to preserve self-modified code on fork, reach out and we
can implement a better solution for you. This gets us unblocked quickly.
This commit is contained in:
Justine Tunney 2025-01-05 09:24:17 -08:00
parent f71f61cd40
commit 29eb7e67bb
No known key found for this signature in database
GPG key ID: BE714B4575D6E328
11 changed files with 66 additions and 34 deletions

View file

@ -80,7 +80,8 @@ textwindows int IsWindowsExecutable(int64_t handle, const char16_t *path) {
uint32_t got; uint32_t got;
BLOCK_SIGNALS; BLOCK_SIGNALS;
struct NtOverlapped overlap = {.hEvent = CreateEvent(0, 0, 0, 0)}; struct NtOverlapped overlap = {.hEvent = CreateEvent(0, 0, 0, 0)};
ok = (ReadFile(handle, buf, 2, 0, &overlap) || ok = overlap.hEvent &&
(ReadFile(handle, buf, 2, 0, &overlap) ||
GetLastError() == kNtErrorIoPending) && GetLastError() == kNtErrorIoPending) &&
GetOverlappedResult(handle, &overlap, &got, true); GetOverlappedResult(handle, &overlap, &got, true);
CloseHandle(overlap.hEvent); CloseHandle(overlap.hEvent);

View file

@ -22,7 +22,6 @@
#include "libc/fmt/itoa.h" #include "libc/fmt/itoa.h"
#include "libc/intrin/fds.h" #include "libc/intrin/fds.h"
#include "libc/intrin/maps.h" #include "libc/intrin/maps.h"
#include "libc/intrin/strace.h"
#include "libc/mem/mem.h" #include "libc/mem/mem.h"
#include "libc/nt/files.h" #include "libc/nt/files.h"
#include "libc/nt/runtime.h" #include "libc/nt/runtime.h"
@ -68,7 +67,6 @@ textwindows void __undescribe_fds(int64_t hCreatorProcess,
uint32_t dwExplicitHandleCount) { uint32_t dwExplicitHandleCount) {
if (lpExplicitHandles) { if (lpExplicitHandles) {
for (uint32_t i = 0; i < dwExplicitHandleCount; ++i) { for (uint32_t i = 0; i < dwExplicitHandleCount; ++i) {
STRACE("close handle %lx %lx", hCreatorProcess, lpExplicitHandles[i]);
DuplicateHandle(hCreatorProcess, lpExplicitHandles[i], 0, 0, 0, false, DuplicateHandle(hCreatorProcess, lpExplicitHandles[i], 0, 0, 0, false,
kNtDuplicateCloseSource); kNtDuplicateCloseSource);
} }
@ -127,7 +125,6 @@ textwindows char *__describe_fds(const struct Fd *fds, size_t fdslen,
for (uint32_t i = 0; i < 3; ++i) for (uint32_t i = 0; i < 3; ++i)
if (lpStartupInfo->stdiofds[i] == f->handle) if (lpStartupInfo->stdiofds[i] == f->handle)
lpStartupInfo->stdiofds[i] = handle; lpStartupInfo->stdiofds[i] = handle;
STRACE("added handle %lx", handle);
handles[hi++] = handle; handles[hi++] = handle;
// get shared memory handle for the file offset pointer // get shared memory handle for the file offset pointer
@ -144,7 +141,6 @@ textwindows char *__describe_fds(const struct Fd *fds, size_t fdslen,
__winerr(); __winerr();
goto OnFailure; goto OnFailure;
} }
STRACE("added handle %lx", shand);
handles[hi++] = shand; handles[hi++] = shand;
} }

View file

@ -70,11 +70,8 @@ textwindows int sys_execve_nt(const char *program, char *const argv[],
// execve() needs to be @asyncsignalsafe // execve() needs to be @asyncsignalsafe
sigset_t sigmask = __sig_block(); sigset_t sigmask = __sig_block();
STRACE("execve step #1");
_pthread_mutex_lock(&__sig_worker_lock); // order matters _pthread_mutex_lock(&__sig_worker_lock); // order matters
STRACE("execve step #2");
_pthread_lock(); // order matters _pthread_lock(); // order matters
STRACE("execve step #3");
// new process should be a child of our parent // new process should be a child of our parent
int64_t hParentProcess = int64_t hParentProcess =
@ -83,8 +80,6 @@ textwindows int sys_execve_nt(const char *program, char *const argv[],
sys_getppid_nt_win32) sys_getppid_nt_win32)
: 0; : 0;
STRACE("execve step #4");
// inherit pid // inherit pid
char pidvar[11 + 21]; char pidvar[11 + 21];
FormatUint64(stpcpy(pidvar, "_COSMO_PID="), __pid); FormatUint64(stpcpy(pidvar, "_COSMO_PID="), __pid);
@ -103,8 +98,6 @@ textwindows int sys_execve_nt(const char *program, char *const argv[],
setenv("_COSMO_PPID", ppidvar, true); setenv("_COSMO_PPID", ppidvar, true);
} }
STRACE("execve step #5");
// define stdio handles for the spawned subprocess // define stdio handles for the spawned subprocess
struct NtStartupInfo si = { struct NtStartupInfo si = {
.cb = sizeof(struct NtStartupInfo), .cb = sizeof(struct NtStartupInfo),
@ -118,8 +111,6 @@ textwindows int sys_execve_nt(const char *program, char *const argv[],
} }
} }
STRACE("execve step #6");
// which process is responsible for spawning the child? // which process is responsible for spawning the child?
int64_t hCreatorProcess; int64_t hCreatorProcess;
if (hParentProcess) { if (hParentProcess) {
@ -140,25 +131,19 @@ textwindows int sys_execve_nt(const char *program, char *const argv[],
return -1; return -1;
} }
STRACE("execve step #7");
// inherit pending signals // inherit pending signals
atomic_fetch_or_explicit( atomic_fetch_or_explicit(
__sig.process, __sig.process,
atomic_load_explicit(&__get_tls()->tib_sigpending, memory_order_acquire), atomic_load_explicit(&__get_tls()->tib_sigpending, memory_order_acquire),
memory_order_release); memory_order_release);
STRACE("execve step #8");
// launch the process // launch the process
struct NtProcessInformation pi; struct NtProcessInformation pi;
int rc = ntspawn(&(struct NtSpawnArgs){ int rc = ntspawn(&(struct NtSpawnArgs){
AT_FDCWD, program, argv, envp, AT_FDCWD, program, argv, envp,
(char *[]){fdspec, maskvar, pidvar, ppidvar, 0}, 0, 0, hCreatorProcess, (char *[]){fdspec, maskvar, pidvar, ppidvar, 0}, 0, 0, hCreatorProcess,
lpExplicitHandles, dwExplicitHandleCount, &si, &pi}); lpExplicitHandles, dwExplicitHandleCount, &si, &pi});
STRACE("execve step #9");
__undescribe_fds(hCreatorProcess, lpExplicitHandles, dwExplicitHandleCount); __undescribe_fds(hCreatorProcess, lpExplicitHandles, dwExplicitHandleCount);
STRACE("execve step #10");
if (rc == -1) { if (rc == -1) {
free(fdspec); free(fdspec);
if (hParentProcess) if (hParentProcess)
@ -177,11 +162,9 @@ textwindows int sys_execve_nt(const char *program, char *const argv[],
int64_t handle; int64_t handle;
if (DuplicateHandle(GetCurrentProcess(), pi.hProcess, hParentProcess, if (DuplicateHandle(GetCurrentProcess(), pi.hProcess, hParentProcess,
&handle, 0, false, kNtDuplicateSameAccess)) { &handle, 0, false, kNtDuplicateSameAccess)) {
STRACE("execve step #11");
unassert(!(handle & 0xFFFFFFFFFF000000)); unassert(!(handle & 0xFFFFFFFFFF000000));
__imp_TerminateProcess(-1, 0x23000000u | handle); __imp_TerminateProcess(-1, 0x23000000u | handle);
} else { } else {
STRACE("execve step #12");
// TODO(jart): Why does `make loc` print this? // TODO(jart): Why does `make loc` print this?
// kprintf("DuplicateHandle failed w/ %d\n", GetLastError()); // kprintf("DuplicateHandle failed w/ %d\n", GetLastError());
__imp_TerminateProcess(-1, ECHILD); __imp_TerminateProcess(-1, ECHILD);

View file

@ -90,6 +90,7 @@ textwindows static void sys_fork_nt_child(void) {
// setup runtime // setup runtime
__klog_handle = 0; __klog_handle = 0;
__tls_index = __imp_TlsAlloc(); __tls_index = __imp_TlsAlloc();
__morph_tls();
__set_tls_win32(__winmain_tib); __set_tls_win32(__winmain_tib);
__tls_enabled = true; __tls_enabled = true;
@ -241,6 +242,9 @@ textwindows static int sys_fork_nt_parent(uint32_t dwCreationFlags) {
} }
if ((map->flags & MAP_NOFORK) && (map->flags & MAP_TYPE) == MAP_FILE) { if ((map->flags & MAP_NOFORK) && (map->flags & MAP_TYPE) == MAP_FILE) {
// portable executable segment // portable executable segment
if (map->prot & PROT_EXEC)
// TODO(jart): write a __remorph_tls() function
continue;
if (!(map->prot & PROT_WRITE)) { if (!(map->prot & PROT_WRITE)) {
uint32_t child_old_protect; uint32_t child_old_protect;
ok = ok && !!VirtualProtectEx(procinfo.hProcess, map->addr, allocsize, ok = ok && !!VirtualProtectEx(procinfo.hProcess, map->addr, allocsize,

View file

@ -298,6 +298,11 @@ int _fork(uint32_t dwCreationFlags) {
} }
} }
// reactivate ftrace
/* if (ftrace_stackdigs) */
/* if (_weaken(ftrace_install)) */
/* _weaken(ftrace_install)(); */
STRACE("fork() → 0 (child of %d)", ppid_cosmo); STRACE("fork() → 0 (child of %d)", ppid_cosmo);
} else { } else {
// this is the parent process // this is the parent process

View file

@ -131,14 +131,18 @@ textwindows static int __proc_wait(int pid, int *wstatus, int options,
// perform blocking operation // perform blocking operation
uint32_t wi; uint32_t wi;
uintptr_t event; uintptr_t event;
if ((event = CreateEvent(0, 0, 0, 0))) {
struct PosixThread *pt = _pthread_self(); struct PosixThread *pt = _pthread_self();
pt->pt_event = event;
pt->pt_blkmask = waitmask; pt->pt_blkmask = waitmask;
pt->pt_event = event = CreateEvent(0, 0, 0, 0);
atomic_store_explicit(&pt->pt_blocker, PT_BLOCKER_EVENT, atomic_store_explicit(&pt->pt_blocker, PT_BLOCKER_EVENT,
memory_order_release); memory_order_release);
wi = WaitForMultipleObjects(2, (intptr_t[2]){hWaitObject, event}, 0, -1u); wi = WaitForMultipleObjects(2, (intptr_t[2]){hWaitObject, event}, 0, -1u);
atomic_store_explicit(&pt->pt_blocker, 0, memory_order_release); atomic_store_explicit(&pt->pt_blocker, 0, memory_order_release);
CloseHandle(event); CloseHandle(event);
} else {
wi = -1u;
}
// log warning if handle unexpectedly closed // log warning if handle unexpectedly closed
if (wi & kNtWaitAbandoned) { if (wi & kNtWaitAbandoned) {

View file

@ -92,9 +92,10 @@ textwindows dontinline static ssize_t sys_sendfile_nt(
} }
struct NtOverlapped ov = {.hEvent = WSACreateEvent(), .Pointer = offset}; struct NtOverlapped ov = {.hEvent = WSACreateEvent(), .Pointer = offset};
cosmo_once(&g_transmitfile.once, transmitfile_init); cosmo_once(&g_transmitfile.once, transmitfile_init);
if (g_transmitfile.lpTransmitFile(oh, ih, uptobytes, 0, &ov, 0, 0) || if (ov.hEvent &&
(g_transmitfile.lpTransmitFile(oh, ih, uptobytes, 0, &ov, 0, 0) ||
WSAGetLastError() == kNtErrorIoPending || WSAGetLastError() == kNtErrorIoPending ||
WSAGetLastError() == WSAEINPROGRESS) { WSAGetLastError() == WSAEINPROGRESS)) {
if (WSAGetOverlappedResult(oh, &ov, &uptobytes, true, &flags)) { if (WSAGetOverlappedResult(oh, &ov, &uptobytes, true, &flags)) {
rc = uptobytes; rc = uptobytes;
if (opt_in_out_inoffset) { if (opt_in_out_inoffset) {

View file

@ -31,6 +31,7 @@ TEST_LIBC_PROC_DIRECTDEPS = \
LIBC_NT_KERNEL32 \ LIBC_NT_KERNEL32 \
LIBC_PROC \ LIBC_PROC \
LIBC_RUNTIME \ LIBC_RUNTIME \
LIBC_LOG \
LIBC_STDIO \ LIBC_STDIO \
LIBC_STR \ LIBC_STR \
LIBC_SYSV \ LIBC_SYSV \
@ -90,6 +91,7 @@ o/$(MODE)/test/libc/proc/execve_test.dbg: \
o/$(MODE)/test/libc/proc/execve_test.o \ o/$(MODE)/test/libc/proc/execve_test.o \
o/$(MODE)/test/libc/calls/life-nomod.zip.o \ o/$(MODE)/test/libc/calls/life-nomod.zip.o \
o/$(MODE)/test/libc/proc/execve_test_prog1.zip.o \ o/$(MODE)/test/libc/proc/execve_test_prog1.zip.o \
o/$(MODE)/test/libc/proc/execve_test_prog2.zip.o \
o/$(MODE)/test/libc/mem/prog/life.elf.zip.o \ o/$(MODE)/test/libc/mem/prog/life.elf.zip.o \
o/$(MODE)/test/libc/mem/prog/sock.elf.zip.o \ o/$(MODE)/test/libc/mem/prog/sock.elf.zip.o \
o/$(MODE)/test/libc/proc/proc.pkg \ o/$(MODE)/test/libc/proc/proc.pkg \
@ -119,6 +121,7 @@ o/$(MODE)/test/libc/proc/life.dbg: \
o/$(MODE)/test/libc/proc/life.zip.o \ o/$(MODE)/test/libc/proc/life.zip.o \
o/$(MODE)/test/libc/proc/execve_test_prog1.zip.o \ o/$(MODE)/test/libc/proc/execve_test_prog1.zip.o \
o/$(MODE)/test/libc/proc/execve_test_prog2.zip.o \
o/$(MODE)/test/libc/proc/life-pe.zip.o: private \ o/$(MODE)/test/libc/proc/life-pe.zip.o: private \
ZIPOBJ_FLAGS += \ ZIPOBJ_FLAGS += \
-B -B

View file

@ -53,6 +53,7 @@ TEST(execve, testArgPassing) {
char ibuf[12], buf[8]; char ibuf[12], buf[8];
const char *prog = "./execve_test_prog1"; const char *prog = "./execve_test_prog1";
testlib_extract("/zip/execve_test_prog1", prog, 0755); testlib_extract("/zip/execve_test_prog1", prog, 0755);
testlib_extract("/zip/execve_test_prog2", "execve_test_prog2", 0755);
for (i = 0; i < N; ++i) { for (i = 0; i < N; ++i) {
FormatInt32(ibuf, i); FormatInt32(ibuf, i);
GenBuf(buf, i); GenBuf(buf, i);

View file

@ -18,6 +18,7 @@
*/ */
#include "libc/calls/calls.h" #include "libc/calls/calls.h"
#include "libc/fmt/conv.h" #include "libc/fmt/conv.h"
#include "libc/runtime/runtime.h"
#include "libc/str/str.h" #include "libc/str/str.h"
void GenBuf(char buf[8], int x) { void GenBuf(char buf[8], int x) {
@ -40,5 +41,15 @@ int main(int argc, char *argv[]) {
tinyprint(2, "error: buf check failed\n", NULL); tinyprint(2, "error: buf check failed\n", NULL);
return 10; return 10;
} }
const char *prog = "./execve_test_prog2";
if (!fork()) {
execl(prog, prog, NULL);
_Exit(127);
}
int ws;
if (wait(&ws) == -1)
return 30;
if (ws)
return 31;
return 0; return 0;
} }

View file

@ -0,0 +1,23 @@
/*-*- mode:c;indent-tabs-mode:nil;c-basic-offset:2;tab-width:8;coding:utf-8 -*-│
vi: set et ft=c ts=2 sts=2 sw=2 fenc=utf-8 :vi
Copyright 2024 Justine Alexandra Roberts Tunney
Permission to use, copy, modify, and/or distribute this software for
any purpose with or without fee is hereby granted, provided that the
above copyright notice and this permission notice appear in all copies.
THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL
WARRANTIES WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED
WARRANTIES OF MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE
AUTHOR BE LIABLE FOR ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL
DAMAGES OR ANY DAMAGES WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR
PROFITS, WHETHER IN AN ACTION OF CONTRACT, NEGLIGENCE OR OTHER
TORTIOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR
PERFORMANCE OF THIS SOFTWARE.
*/
#include "libc/proc/proc.h"
#include "libc/testlib/testlib.h"
int main(int argc, char *argv[]) {
}