From 29eb7e67bbda62eb630c47b4cc515e88a4f13447 Mon Sep 17 00:00:00 2001 From: Justine Tunney Date: Sun, 5 Jan 2025 09:24:17 -0800 Subject: [PATCH] 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. --- libc/calls/winexec.c | 3 ++- libc/proc/describefds.c | 4 ---- libc/proc/execve-nt.greg.c | 19 +------------------ libc/proc/fork-nt.c | 4 ++++ libc/proc/fork.c | 5 +++++ libc/proc/wait4-nt.c | 20 ++++++++++++-------- libc/sock/sendfile.c | 7 ++++--- test/libc/proc/BUILD.mk | 3 +++ test/libc/proc/execve_test.c | 1 + test/libc/proc/execve_test_prog1.c | 11 +++++++++++ test/libc/proc/execve_test_prog2.c | 23 +++++++++++++++++++++++ 11 files changed, 66 insertions(+), 34 deletions(-) create mode 100644 test/libc/proc/execve_test_prog2.c diff --git a/libc/calls/winexec.c b/libc/calls/winexec.c index ca52372c5..429589c10 100644 --- a/libc/calls/winexec.c +++ b/libc/calls/winexec.c @@ -80,7 +80,8 @@ textwindows int IsWindowsExecutable(int64_t handle, const char16_t *path) { uint32_t got; BLOCK_SIGNALS; 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) && GetOverlappedResult(handle, &overlap, &got, true); CloseHandle(overlap.hEvent); diff --git a/libc/proc/describefds.c b/libc/proc/describefds.c index 847817595..4bc203fe3 100644 --- a/libc/proc/describefds.c +++ b/libc/proc/describefds.c @@ -22,7 +22,6 @@ #include "libc/fmt/itoa.h" #include "libc/intrin/fds.h" #include "libc/intrin/maps.h" -#include "libc/intrin/strace.h" #include "libc/mem/mem.h" #include "libc/nt/files.h" #include "libc/nt/runtime.h" @@ -68,7 +67,6 @@ textwindows void __undescribe_fds(int64_t hCreatorProcess, uint32_t dwExplicitHandleCount) { if (lpExplicitHandles) { for (uint32_t i = 0; i < dwExplicitHandleCount; ++i) { - STRACE("close handle %lx %lx", hCreatorProcess, lpExplicitHandles[i]); DuplicateHandle(hCreatorProcess, lpExplicitHandles[i], 0, 0, 0, false, kNtDuplicateCloseSource); } @@ -127,7 +125,6 @@ textwindows char *__describe_fds(const struct Fd *fds, size_t fdslen, for (uint32_t i = 0; i < 3; ++i) if (lpStartupInfo->stdiofds[i] == f->handle) lpStartupInfo->stdiofds[i] = handle; - STRACE("added handle %lx", handle); handles[hi++] = handle; // 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(); goto OnFailure; } - STRACE("added handle %lx", shand); handles[hi++] = shand; } diff --git a/libc/proc/execve-nt.greg.c b/libc/proc/execve-nt.greg.c index 3b8aaa766..2a65d4f9e 100644 --- a/libc/proc/execve-nt.greg.c +++ b/libc/proc/execve-nt.greg.c @@ -70,11 +70,8 @@ textwindows int sys_execve_nt(const char *program, char *const argv[], // execve() needs to be @asyncsignalsafe sigset_t sigmask = __sig_block(); - STRACE("execve step #1"); _pthread_mutex_lock(&__sig_worker_lock); // order matters - STRACE("execve step #2"); - _pthread_lock(); // order matters - STRACE("execve step #3"); + _pthread_lock(); // order matters // new process should be a child of our parent int64_t hParentProcess = @@ -83,8 +80,6 @@ textwindows int sys_execve_nt(const char *program, char *const argv[], sys_getppid_nt_win32) : 0; - STRACE("execve step #4"); - // inherit pid char pidvar[11 + 21]; 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); } - STRACE("execve step #5"); - // define stdio handles for the spawned subprocess struct NtStartupInfo si = { .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? int64_t hCreatorProcess; if (hParentProcess) { @@ -140,25 +131,19 @@ textwindows int sys_execve_nt(const char *program, char *const argv[], return -1; } - STRACE("execve step #7"); - // inherit pending signals atomic_fetch_or_explicit( __sig.process, atomic_load_explicit(&__get_tls()->tib_sigpending, memory_order_acquire), memory_order_release); - STRACE("execve step #8"); - // launch the process struct NtProcessInformation pi; int rc = ntspawn(&(struct NtSpawnArgs){ AT_FDCWD, program, argv, envp, (char *[]){fdspec, maskvar, pidvar, ppidvar, 0}, 0, 0, hCreatorProcess, lpExplicitHandles, dwExplicitHandleCount, &si, &pi}); - STRACE("execve step #9"); __undescribe_fds(hCreatorProcess, lpExplicitHandles, dwExplicitHandleCount); - STRACE("execve step #10"); if (rc == -1) { free(fdspec); if (hParentProcess) @@ -177,11 +162,9 @@ textwindows int sys_execve_nt(const char *program, char *const argv[], int64_t handle; if (DuplicateHandle(GetCurrentProcess(), pi.hProcess, hParentProcess, &handle, 0, false, kNtDuplicateSameAccess)) { - STRACE("execve step #11"); unassert(!(handle & 0xFFFFFFFFFF000000)); __imp_TerminateProcess(-1, 0x23000000u | handle); } else { - STRACE("execve step #12"); // TODO(jart): Why does `make loc` print this? // kprintf("DuplicateHandle failed w/ %d\n", GetLastError()); __imp_TerminateProcess(-1, ECHILD); diff --git a/libc/proc/fork-nt.c b/libc/proc/fork-nt.c index 4e0679b23..7f0ddca2d 100644 --- a/libc/proc/fork-nt.c +++ b/libc/proc/fork-nt.c @@ -90,6 +90,7 @@ textwindows static void sys_fork_nt_child(void) { // setup runtime __klog_handle = 0; __tls_index = __imp_TlsAlloc(); + __morph_tls(); __set_tls_win32(__winmain_tib); __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) { // portable executable segment + if (map->prot & PROT_EXEC) + // TODO(jart): write a __remorph_tls() function + continue; if (!(map->prot & PROT_WRITE)) { uint32_t child_old_protect; ok = ok && !!VirtualProtectEx(procinfo.hProcess, map->addr, allocsize, diff --git a/libc/proc/fork.c b/libc/proc/fork.c index eab5cfb09..81cb09b91 100644 --- a/libc/proc/fork.c +++ b/libc/proc/fork.c @@ -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); } else { // this is the parent process diff --git a/libc/proc/wait4-nt.c b/libc/proc/wait4-nt.c index 8c17f09e0..fe8e0d85d 100644 --- a/libc/proc/wait4-nt.c +++ b/libc/proc/wait4-nt.c @@ -131,14 +131,18 @@ textwindows static int __proc_wait(int pid, int *wstatus, int options, // perform blocking operation uint32_t wi; uintptr_t event; - struct PosixThread *pt = _pthread_self(); - pt->pt_blkmask = waitmask; - pt->pt_event = event = CreateEvent(0, 0, 0, 0); - atomic_store_explicit(&pt->pt_blocker, PT_BLOCKER_EVENT, - memory_order_release); - wi = WaitForMultipleObjects(2, (intptr_t[2]){hWaitObject, event}, 0, -1u); - atomic_store_explicit(&pt->pt_blocker, 0, memory_order_release); - CloseHandle(event); + if ((event = CreateEvent(0, 0, 0, 0))) { + struct PosixThread *pt = _pthread_self(); + pt->pt_event = event; + pt->pt_blkmask = waitmask; + atomic_store_explicit(&pt->pt_blocker, PT_BLOCKER_EVENT, + memory_order_release); + wi = WaitForMultipleObjects(2, (intptr_t[2]){hWaitObject, event}, 0, -1u); + atomic_store_explicit(&pt->pt_blocker, 0, memory_order_release); + CloseHandle(event); + } else { + wi = -1u; + } // log warning if handle unexpectedly closed if (wi & kNtWaitAbandoned) { diff --git a/libc/sock/sendfile.c b/libc/sock/sendfile.c index 17fc36b6a..15a5c03cf 100644 --- a/libc/sock/sendfile.c +++ b/libc/sock/sendfile.c @@ -92,9 +92,10 @@ textwindows dontinline static ssize_t sys_sendfile_nt( } struct NtOverlapped ov = {.hEvent = WSACreateEvent(), .Pointer = offset}; cosmo_once(&g_transmitfile.once, transmitfile_init); - if (g_transmitfile.lpTransmitFile(oh, ih, uptobytes, 0, &ov, 0, 0) || - WSAGetLastError() == kNtErrorIoPending || - WSAGetLastError() == WSAEINPROGRESS) { + if (ov.hEvent && + (g_transmitfile.lpTransmitFile(oh, ih, uptobytes, 0, &ov, 0, 0) || + WSAGetLastError() == kNtErrorIoPending || + WSAGetLastError() == WSAEINPROGRESS)) { if (WSAGetOverlappedResult(oh, &ov, &uptobytes, true, &flags)) { rc = uptobytes; if (opt_in_out_inoffset) { diff --git a/test/libc/proc/BUILD.mk b/test/libc/proc/BUILD.mk index 1664f026a..4175234ec 100644 --- a/test/libc/proc/BUILD.mk +++ b/test/libc/proc/BUILD.mk @@ -31,6 +31,7 @@ TEST_LIBC_PROC_DIRECTDEPS = \ LIBC_NT_KERNEL32 \ LIBC_PROC \ LIBC_RUNTIME \ + LIBC_LOG \ LIBC_STDIO \ LIBC_STR \ 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/calls/life-nomod.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/sock.elf.zip.o \ 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/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 \ ZIPOBJ_FLAGS += \ -B diff --git a/test/libc/proc/execve_test.c b/test/libc/proc/execve_test.c index 01573483e..58d7680f5 100644 --- a/test/libc/proc/execve_test.c +++ b/test/libc/proc/execve_test.c @@ -53,6 +53,7 @@ TEST(execve, testArgPassing) { char ibuf[12], buf[8]; const char *prog = "./execve_test_prog1"; testlib_extract("/zip/execve_test_prog1", prog, 0755); + testlib_extract("/zip/execve_test_prog2", "execve_test_prog2", 0755); for (i = 0; i < N; ++i) { FormatInt32(ibuf, i); GenBuf(buf, i); diff --git a/test/libc/proc/execve_test_prog1.c b/test/libc/proc/execve_test_prog1.c index 5a1ea77e8..901c9b6bc 100644 --- a/test/libc/proc/execve_test_prog1.c +++ b/test/libc/proc/execve_test_prog1.c @@ -18,6 +18,7 @@ ╚─────────────────────────────────────────────────────────────────────────────*/ #include "libc/calls/calls.h" #include "libc/fmt/conv.h" +#include "libc/runtime/runtime.h" #include "libc/str/str.h" 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); 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; } diff --git a/test/libc/proc/execve_test_prog2.c b/test/libc/proc/execve_test_prog2.c new file mode 100644 index 000000000..2369ec9c9 --- /dev/null +++ b/test/libc/proc/execve_test_prog2.c @@ -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[]) { +}