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`.
This commit is contained in:
Jōshin 2023-12-14 16:50:06 -05:00
parent 79e5ca4a24
commit 21b4d218d7
No known key found for this signature in database
3 changed files with 17 additions and 6 deletions

View file

@ -21,6 +21,7 @@
#include "libc/calls/internal.h" #include "libc/calls/internal.h"
#include "libc/calls/state.internal.h" #include "libc/calls/state.internal.h"
#include "libc/calls/struct/fd.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-nt.internal.h"
#include "libc/calls/syscall-sysv.internal.h" #include "libc/calls/syscall-sysv.internal.h"
#include "libc/dce.h" #include "libc/dce.h"
@ -91,8 +92,18 @@ static int close_impl(int fd) {
* @vforksafe * @vforksafe
*/ */
int close(int fd) { int close(int fd) {
int rc = close_impl(fd); int rc;
if (!__vforked) __releasefd(fd); 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); STRACE("close(%d) → %d% m", fd, rc);
return rc; return rc;
} }

View file

@ -235,21 +235,20 @@ void __zipos_postdup(int oldfd, int newfd) {
if (oldfd == newfd) { if (oldfd == newfd) {
return; return;
} }
// XXX signals
__fds_lock();
if (__isfdkind(newfd, kFdZip)) { if (__isfdkind(newfd, kFdZip)) {
__zipos_free((struct ZiposHandle *)(intptr_t)g_fds.p[newfd].handle); __zipos_free((struct ZiposHandle *)(intptr_t)g_fds.p[newfd].handle);
if (!__isfdkind(oldfd, kFdZip)) { if (!__isfdkind(oldfd, kFdZip)) {
__fds_lock();
bzero(g_fds.p + newfd, sizeof(*g_fds.p)); bzero(g_fds.p + newfd, sizeof(*g_fds.p));
__fds_unlock();
} }
} }
if (__isfdkind(oldfd, kFdZip)) { if (__isfdkind(oldfd, kFdZip)) {
__zipos_keep((struct ZiposHandle *)(intptr_t)g_fds.p[oldfd].handle); __zipos_keep((struct ZiposHandle *)(intptr_t)g_fds.p[oldfd].handle);
__fds_lock();
__ensurefds_unlocked(newfd); __ensurefds_unlocked(newfd);
g_fds.p[newfd] = g_fds.p[oldfd]; g_fds.p[newfd] = g_fds.p[oldfd];
__fds_unlock();
} }
__fds_unlock();
} }
/** /**

View file

@ -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 (b) memcpy(iov[i].iov_base, h->mem + y, b);
} }
if (opt_offset == -1) { if (opt_offset == -1) {
unassert(y != SIZE_MAX);
atomic_store_explicit(&h->pos, y, memory_order_release); atomic_store_explicit(&h->pos, y, memory_order_release);
} }
return y - x; return y - x;