From dc1afc968bb7fa1fcf8db416f25082c4381e0367 Mon Sep 17 00:00:00 2001 From: Justine Tunney Date: Sat, 12 Oct 2024 15:26:32 -0700 Subject: [PATCH] Fix fork() crash on Windows On Windows, sometimes fork() could crash with message likes: fork() ViewOrDie(170000) failed with win32 error 487 This is due to a bug in our file descriptor inheritance. We have cursors which are shared between processes. They let us track the file positions of read() and write() operations. At startup they were being mmap()ed to memory addresses that were assigned by WIN32. That's bad because Windows likes to give us memory addresses beneath the program image in the first 4mb range that are likely to conflict with other assignments. That ended up causing problems because fork() needs to be able to assume that a map will be possible to resurrect at the same address. But for one reason or another, Windows libraries we don't control could sneak allocations into the memory space that overlap with these mappings. This change solves it by choosing a random memory address instead when mapping cursor objects. --- libc/intrin/fds.c | 8 ++++- libc/intrin/printmapswin32.c | 65 ++++++++++++++++++++++++++++++++++++ libc/proc/fork-nt.c | 4 +-- libc/runtime/runtime.h | 1 + 4 files changed, 74 insertions(+), 4 deletions(-) create mode 100644 libc/intrin/printmapswin32.c diff --git a/libc/intrin/fds.c b/libc/intrin/fds.c index d2883cccf..67e610bfc 100644 --- a/libc/intrin/fds.c +++ b/libc/intrin/fds.c @@ -129,6 +129,7 @@ textstartup void __init_fds(int argc, char **argv, char **envp) { if (IsWindows()) { const char *fdspec; if ((fdspec = getenv("_COSMO_FDS_V2"))) { + char *smaddr = 0; unsetenv("_COSMO_FDS"); unsetenv("_COSMO_FDS_V2"); for (;;) { @@ -171,8 +172,13 @@ textstartup void __init_fds(int argc, char **argv, char **envp) { if (shand) { struct Map *map; struct CursorShared *shared; + if (!smaddr) { + smaddr = __maps_randaddr(); + } else { + smaddr += 65536; + } if ((shared = MapViewOfFileEx(shand, kNtFileMapWrite, 0, 0, - sizeof(struct CursorShared), 0))) { + sizeof(struct CursorShared), smaddr))) { if ((f->cursor = _mapanon(sizeof(struct Cursor)))) { f->cursor->shared = shared; if ((map = __maps_alloc())) { diff --git a/libc/intrin/printmapswin32.c b/libc/intrin/printmapswin32.c new file mode 100644 index 000000000..65fbcd1e3 --- /dev/null +++ b/libc/intrin/printmapswin32.c @@ -0,0 +1,65 @@ +/*-*- 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/fmt/conv.h" +#include "libc/intrin/describeflags.h" +#include "libc/intrin/kprintf.h" +#include "libc/macros.h" +#include "libc/nt/enum/memflags.h" +#include "libc/nt/memory.h" +#include "libc/runtime/runtime.h" +#include "libc/str/str.h" + +static const struct DescribeFlags kNtMemState[] = { + {kNtMemCommit, "Commit"}, // + {kNtMemFree, "Free"}, // + {kNtMemReserve, "Reserve"}, // +}; + +const char *DescribeNtMemState(char buf[64], uint32_t x) { + return _DescribeFlags(buf, 64, kNtMemState, ARRAYLEN(kNtMemState), "kNtMem", + x); +} + +static const struct DescribeFlags kNtMemType[] = { + {kNtMemImage, "Image"}, // + {kNtMemMapped, "Mapped"}, // + {kNtMemPrivate, "Private"}, // +}; + +const char *DescribeNtMemType(char buf[64], uint32_t x) { + return _DescribeFlags(buf, 64, kNtMemType, ARRAYLEN(kNtMemType), "kNtMem", x); +} + +void __print_maps_win32(void) { + char *p, b[5][64]; + struct NtMemoryBasicInformation mi; + kprintf("%-12s %-12s %10s %16s %16s %32s %32s\n", "Allocation", "BaseAddress", + "RegionSize", "State", "Type", "AllocationProtect", "Protect"); + for (p = 0;; p = (char *)mi.BaseAddress + mi.RegionSize) { + bzero(&mi, sizeof(mi)); + if (!VirtualQuery(p, &mi, sizeof(mi))) + break; + sizefmt(b[0], mi.RegionSize, 1024); + kprintf("%.12lx %.12lx %10s %16s %16s %32s %32s\n", mi.AllocationBase, + mi.BaseAddress, b[0], DescribeNtMemState(b[1], mi.State), + DescribeNtMemType(b[2], mi.Type), + _DescribeNtPageFlags(b[3], mi.AllocationProtect), + _DescribeNtPageFlags(b[4], mi.Protect)); + } +} diff --git a/libc/proc/fork-nt.c b/libc/proc/fork-nt.c index f889d3e8b..ce5907a8a 100644 --- a/libc/proc/fork-nt.c +++ b/libc/proc/fork-nt.c @@ -125,11 +125,9 @@ static dontinline textwindows ssize_t ForkIo2( static dontinline textwindows bool WriteAll(int64_t h, void *buf, size_t n) { bool ok; ok = ForkIo2(h, buf, n, (void *)WriteFile, "WriteFile", false) != -1; - if (!ok) { + if (!ok) STRACE("fork() failed in parent due to WriteAll(%ld, %p, %'zu) → %u", h, buf, n, GetLastError()); - __print_maps(0); - } return ok; } diff --git a/libc/runtime/runtime.h b/libc/runtime/runtime.h index 452125bcb..58fde8c23 100644 --- a/libc/runtime/runtime.h +++ b/libc/runtime/runtime.h @@ -95,6 +95,7 @@ int ftrace_install(void) libcesque; int ftrace_enabled(int) libcesque; int strace_enabled(int) libcesque; void __print_maps(size_t) libcesque; +void __print_maps_win32(void) libcesque; void __printargs(const char *) libcesque; /* builtin sh-like system/popen dsl */ int _cocmd(int, char **, char **) libcesque;