From b275e664ecf22d9bf538b221c4e56ff6d2e381a1 Mon Sep 17 00:00:00 2001 From: Gavin Hayes Date: Wed, 22 Feb 2023 21:57:36 -0500 Subject: [PATCH] Make ZipOS mmap safer (#735) - It now runs entirely under __mmi_lock - Hide implementation strace --- libc/runtime/internal.h | 3 +++ libc/runtime/mmap.c | 19 +++++++++-------- libc/runtime/munmap.c | 4 +--- libc/zipos/mmap.c | 41 ++++++++++++++++++++++++------------- libc/zipos/zipos.S | 2 +- libc/zipos/zipos.internal.h | 4 ++-- 6 files changed, 44 insertions(+), 29 deletions(-) diff --git a/libc/runtime/internal.h b/libc/runtime/internal.h index 070106a2d..ba5fa93fe 100644 --- a/libc/runtime/internal.h +++ b/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) */ diff --git a/libc/runtime/mmap.c b/libc/runtime/mmap.c index bad8023ee..e9c274f1e 100644 --- a/libc/runtime/mmap.c +++ b/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, - int fd, int64_t off) { +noasan inline void *Mmap(void *addr, size_t size, int prot, int flags, int fd, + 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; - if (__isfdkind(fd, kFdZip)) { - return _weaken(__zipos_mmap)( - addr, size, prot, flags, - (struct ZiposHandle *)(intptr_t)g_fds.p[fd].handle, off); - } + size_t toto = 0; #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 diff --git a/libc/runtime/munmap.c b/libc/runtime/munmap.c index fe3fff811..b1baac6b2 100644 --- a/libc/runtime/munmap.c +++ b/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; diff --git a/libc/zipos/mmap.c b/libc/zipos/mmap.c index 65255d57b..bddd1b743 100644 --- a/libc/zipos/mmap.c +++ b/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, - struct ZiposHandle *h, int64_t off) { +noasan void *__zipos_Mmap(void *addr, size_t size, int prot, int flags, + 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); - if ((beforeOffset == -1) || - (__zipos_read(h, &(struct iovec){outAddr, size}, 1, off) == -1) || - (__zipos_lseek(h, beforeOffset, SEEK_SET) == -1) || - ((prot != tempProt) && (mprotect(outAddr, size, prot) == -1))) { - const int e = errno; - munmap(outAddr, size); - errno = e; - return MAP_FAILED; - } - return outAddr; + do { + if (__zipos_read(h, &(struct iovec){outAddr, size}, 1, off) == -1) { + strace_enabled(-1); + break; + } else if (prot != tempProt) { + strace_enabled(-1); + if (mprotect(outAddr, size, prot) == -1) { + break; + } + strace_enabled(+1); + } + return outAddr; + } while (0); + const int e = errno; + Munmap(outAddr, size); + errno = e; + strace_enabled(+1); + return MAP_FAILED; } diff --git a/libc/zipos/zipos.S b/libc/zipos/zipos.S index e0ef33ff3..281788129 100644 --- a/libc/zipos/zipos.S +++ b/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 diff --git a/libc/zipos/zipos.internal.h b/libc/zipos/zipos.internal.h index d6565e0d5..a77a911ab 100644 --- a/libc/zipos/zipos.internal.h +++ b/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 *, - int64_t) _Hide; +noasan void *__zipos_Mmap(void *, uint64_t, int32_t, int32_t, + struct ZiposHandle *, int64_t) _Hide; #ifdef _NOPL0 #define __zipos_lock() _NOPL0("__threadcalls", __zipos_lock)