Make ZipOS mmap safer (#735)

- It now runs entirely under __mmi_lock
- Hide implementation strace

libc/runtime/internal.h

@@ -44,6 +44,9 @@ int GetDosEnviron(const char16_t *, char *, size_t, char **, size_t);
 bool __intercept_flag(int *, char *[], const char *);
 int sys_mprotect_nt(void *, size_t, int) _Hide;
 int __inflate(void *, size_t, const void *, size_t);
+noasan void *Mmap(void *addr, size_t size, int prot, int flags, int fd,
+                  int64_t off) _Hide;
+noasan int Munmap(char *, size_t) _Hide;
 
 COSMOPOLITAN_C_END_
 #endif /* !(__ASSEMBLER__ + __LINKER__ + 0) */

libc/runtime/mmap.c

@@ -238,8 +238,8 @@ static textwindows dontinline noasan void *MapMemories(char *addr, size_t size,
   return addr;
 }
 
-static noasan inline void *Mmap(void *addr, size_t size, int prot, int flags,
+noasan inline void *Mmap(void *addr, size_t size, int prot, int flags, int fd,
-                                int fd, int64_t off) {
+                         int64_t off) {
   char *p = addr;
   struct DirectMap dm;
   int a, b, i, f, m, n, x;
@@ -485,12 +485,7 @@ static noasan inline void *Mmap(void *addr, size_t size, int prot, int flags,
  */
 void *mmap(void *addr, size_t size, int prot, int flags, int fd, int64_t off) {
   void *res;
-  size_t toto;
+  size_t toto = 0;
-  if (__isfdkind(fd, kFdZip)) {
-    return _weaken(__zipos_mmap)(
-        addr, size, prot, flags,
-        (struct ZiposHandle *)(intptr_t)g_fds.p[fd].handle, off);
-  }
 #if defined(SYSDEBUG) && (_KERNTRACE || _NTTRACE)
   if (IsWindows()) {
     STRACE("mmap(%p, %'zu, %s, %s, %d, %'ld) → ...", addr, size,
@@ -498,7 +493,13 @@ void *mmap(void *addr, size_t size, int prot, int flags, int fd, int64_t off) {
   }
 #endif
   __mmi_lock();
-  res = Mmap(addr, size, prot, flags, fd, off);
+  if (!__isfdkind(fd, kFdZip)) {
+    res = Mmap(addr, size, prot, flags, fd, off);
+  } else {
+    res = _weaken(__zipos_Mmap)(
+        addr, size, prot, flags,
+        (struct ZiposHandle *)(intptr_t)g_fds.p[fd].handle, off);
+  }
 #if SYSDEBUG
   toto = __strace > 0 ? GetMemtrackSize(&_mmi) : 0;
 #endif

libc/runtime/munmap.c

@@ -40,8 +40,6 @@
 #define ADDR(x)    ((int64_t)((uint64_t)(x) << 32) >> 16)
 #define FRAME(x)   ((int)((intptr_t)(x) >> 16))
 
-static noasan int Munmap(char *, size_t);
-
 static noasan void MunmapShadow(char *p, size_t n) {
   intptr_t a, b, x, y;
   KERNTRACE("MunmapShadow(%p, %'zu)", p, n);
@@ -115,7 +113,7 @@ static noasan void MunmapImpl(char *p, size_t n) {
   }
 }
 
-static noasan int Munmap(char *p, size_t n) {
+noasan int Munmap(char *p, size_t n) {
   unsigned i;
   char poison;
   intptr_t a, b, x, y;

libc/zipos/mmap.c

@@ -22,6 +22,7 @@
 #include "libc/errno.h"
 #include "libc/intrin/likely.h"
 #include "libc/intrin/strace.internal.h"
+#include "libc/runtime/internal.h"
 #include "libc/sysv/consts/map.h"
 #include "libc/sysv/consts/prot.h"
 #include "libc/sysv/errfuns.h"
@@ -46,8 +47,8 @@
  *     it does not need to be 64kb aligned.
  * @return virtual base address of new mapping, or MAP_FAILED w/ errno
  */
-void *__zipos_mmap(void *addr, size_t size, int prot, int flags,
+noasan void *__zipos_Mmap(void *addr, size_t size, int prot, int flags,
-                   struct ZiposHandle *h, int64_t off) {
+                          struct ZiposHandle *h, int64_t off) {
   if (!(flags & MAP_PRIVATE) ||
       (flags & ~(MAP_PRIVATE | MAP_FILE | MAP_FIXED | MAP_FIXED_NOREPLACE)) ||
       (!!(flags & MAP_FIXED) ^ !!(flags & MAP_FIXED_NOREPLACE))) {
@@ -56,21 +57,33 @@ void *__zipos_mmap(void *addr, size_t size, int prot, int flags,
     return VIP(einval());
   }
 
+  if (VERY_UNLIKELY(off < 0)) {
+    STRACE("neg off");
+    return VIP(einval());
+  }
+
   const int tempProt = !IsXnu() ? prot | PROT_WRITE : PROT_WRITE;
   void *outAddr =
-      mmap(addr, size, tempProt, (flags & (~MAP_FILE)) | MAP_ANONYMOUS, -1, 0);
+      Mmap(addr, size, tempProt, (flags & (~MAP_FILE)) | MAP_ANONYMOUS, -1, 0);
   if (outAddr == MAP_FAILED) {
     return MAP_FAILED;
   }
-  const int64_t beforeOffset = __zipos_lseek(h, 0, SEEK_CUR);
+  do {
-  if ((beforeOffset == -1) ||
+    if (__zipos_read(h, &(struct iovec){outAddr, size}, 1, off) == -1) {
-      (__zipos_read(h, &(struct iovec){outAddr, size}, 1, off) == -1) ||
+      strace_enabled(-1);
-      (__zipos_lseek(h, beforeOffset, SEEK_SET) == -1) ||
+      break;
-      ((prot != tempProt) && (mprotect(outAddr, size, prot) == -1))) {
+    } else if (prot != tempProt) {
-    const int e = errno;
+      strace_enabled(-1);
-    munmap(outAddr, size);
+      if (mprotect(outAddr, size, prot) == -1) {
-    errno = e;
+        break;
-    return MAP_FAILED;
+      }
-  }
+      strace_enabled(+1);
-  return outAddr;
+    }
+    return outAddr;
+  } while (0);
+  const int e = errno;
+  Munmap(outAddr, size);
+  errno = e;
+  strace_enabled(+1);
+  return MAP_FAILED;
 }

libc/zipos/zipos.S

@@ -34,7 +34,7 @@
 	.yoink	__zipos_read
 	.yoink	__zipos_stat
 	.yoink	__zipos_notat
-	.yoink	__zipos_mmap
+	.yoink	__zipos_Mmap
 
 //	TODO(jart): why does corruption happen when zip has no assets?
 	.yoink	.cosmo

libc/zipos/zipos.internal.h

@@ -48,8 +48,8 @@ ssize_t __zipos_write(struct ZiposHandle *, const struct iovec *, size_t,
 int64_t __zipos_lseek(struct ZiposHandle *, int64_t, unsigned) _Hide;
 int __zipos_fcntl(int, int, uintptr_t) _Hide;
 int __zipos_notat(int, const char *) _Hide;
-void *__zipos_mmap(void *, uint64_t, int32_t, int32_t, struct ZiposHandle *,
+noasan void *__zipos_Mmap(void *, uint64_t, int32_t, int32_t,
-                   int64_t) _Hide;
+                          struct ZiposHandle *, int64_t) _Hide;
 
 #ifdef _NOPL0
 #define __zipos_lock()   _NOPL0("__threadcalls", __zipos_lock)