From 21b4d218d7c5d655293ae31b3d6204e6fed6847b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C5=8Dshin?= <git@wholezero.org>
Date: Thu, 14 Dec 2023 16:50:06 -0500
Subject: [PATCH] Fix zipos deadlock/segfault

Locks `g_fds` for /zip `close()` calls. Moves lock to the top level in
`__zipos_postdup`. The latter needs review for signal blocking
correctness; it is no more wrong than it was before on that front, but
I'm not at all confident that it was ever right.

Added a defensive assert in `__zipos_read`.
---
 libc/calls/close.c        | 15 +++++++++++++--
 libc/runtime/zipos-open.c |  7 +++----
 libc/runtime/zipos-read.c |  1 +
 3 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/libc/calls/close.c b/libc/calls/close.c
index 25a143b6d..95068563e 100644
--- a/libc/calls/close.c
+++ b/libc/calls/close.c
@@ -21,6 +21,7 @@
 #include "libc/calls/internal.h"
 #include "libc/calls/state.internal.h"
 #include "libc/calls/struct/fd.internal.h"
+#include "libc/calls/struct/sigset.internal.h"
 #include "libc/calls/syscall-nt.internal.h"
 #include "libc/calls/syscall-sysv.internal.h"
 #include "libc/dce.h"
@@ -91,8 +92,18 @@ static int close_impl(int fd) {
  * @vforksafe
  */
 int close(int fd) {
-  int rc = close_impl(fd);
-  if (!__vforked) __releasefd(fd);
+  int rc;
+  if (__isfdkind(fd, kFdZip)) {   // XXX IsWindows()?
+    BLOCK_SIGNALS;
+    __fds_lock();
+    rc = close_impl(fd);
+    if (!__vforked) __releasefd(fd);
+    __fds_unlock();
+    ALLOW_SIGNALS;
+  } else {
+    rc = close_impl(fd);
+    if (!__vforked) __releasefd(fd);
+  }
   STRACE("close(%d) → %d% m", fd, rc);
   return rc;
 }
diff --git a/libc/runtime/zipos-open.c b/libc/runtime/zipos-open.c
index 629a558ce..6841e68a9 100644
--- a/libc/runtime/zipos-open.c
+++ b/libc/runtime/zipos-open.c
@@ -235,21 +235,20 @@ void __zipos_postdup(int oldfd, int newfd) {
   if (oldfd == newfd) {
     return;
   }
+  // XXX signals
+  __fds_lock();
   if (__isfdkind(newfd, kFdZip)) {
     __zipos_free((struct ZiposHandle *)(intptr_t)g_fds.p[newfd].handle);
     if (!__isfdkind(oldfd, kFdZip)) {
-      __fds_lock();
       bzero(g_fds.p + newfd, sizeof(*g_fds.p));
-      __fds_unlock();
     }
   }
   if (__isfdkind(oldfd, kFdZip)) {
     __zipos_keep((struct ZiposHandle *)(intptr_t)g_fds.p[oldfd].handle);
-    __fds_lock();
     __ensurefds_unlocked(newfd);
     g_fds.p[newfd] = g_fds.p[oldfd];
-    __fds_unlock();
   }
+  __fds_unlock();
 }
 
 /**
diff --git a/libc/runtime/zipos-read.c b/libc/runtime/zipos-read.c
index 0f375d526..edc6ceab1 100644
--- a/libc/runtime/zipos-read.c
+++ b/libc/runtime/zipos-read.c
@@ -58,6 +58,7 @@ static ssize_t __zipos_read_impl(struct ZiposHandle *h, const struct iovec *iov,
     if (b) memcpy(iov[i].iov_base, h->mem + y, b);
   }
   if (opt_offset == -1) {
+    unassert(y != SIZE_MAX);
     atomic_store_explicit(&h->pos, y, memory_order_release);
   }
   return y - x;