Support read-only memory protection

Blink previously did very little to enforce the memory protections. Read-only protections would oftentimes be ignored as read+write, if convenient. This change ensures that Blink, in all operating modes, will enforce the guest's memory protections, to the greatest extent possible. As an exception, if the guest does not accurately observe the underlying host system page size, then Blink reserves the right to relax permissions, so that programs will actually work, which is more important than imposing restrictions on the program's behavior
jart · Mar 7, 2023 · 815ff2e · 815ff2e
1 parent 37cd5bc
commit 815ff2e
Show file tree

Hide file tree

Showing 16 changed files with 204 additions and 42 deletions.
diff --git a/README.md b/README.md
@@ -854,6 +854,23 @@ Blink uses `SIGSYS` to deliver signals internally. This signal is
 precious to Blink. It's currently not possible for guest applications to
 capture it from external processes.
 
+### Memory Protection
+
+Blink offers guest programs a 48-bit virtual address space with a
+4096-byte page size. When programs are run on (1) host systems that have
+a larger page (e.g. Apple M1, Cygwin), and (2) the linear memory
+optimization is enabled (i.e. you're *not* using `blink -m`) then Blink
+may need to relax memory protections in cases where the memory intervals
+defined by the guest aren't aligned to the host system page size. Is is
+recommended, when calling functions like mmap() and mprotect(), that
+both `addr` and `addr + size` be aliged to the true page size, which
+Blink reports to the guest in `getauxval(AT_PAGESZ)`. This value should
+be obtainable via the portable API `sysconf(_SC_PAGESIZE)` assuming the
+C library implements it correctly. Please note that when Blink is
+running in its fully virtualized mode (i.e. `blink -m`) this concern
+does not apply. That's because Blink will allocate a full system page
+for every 4096 byte page that gets mapped from a file.
+
 ### Self Modifying Code
 
 Blink supports self-modifying code, with some caveats.

diff --git a/blink/blink.1 b/blink/blink.1
@@ -251,6 +251,28 @@ Blink uses
 to deliver signals internally. This signal is
 precious to Blink. It's currently not possible for guest applications to
 capture it from external processes.
+.Ss "Memory Protection"
+Blink offers guest programs a 48-bit virtual address space with a
+4096-byte page size. When programs are run on (1) host systems that have
+a larger page (e.g. Apple M1, Cygwin), and (2) the linear memory
+optimization is enabled (i.e. you're *not* using
+.Li blink -m
+) then Blink may need to relax memory protections in cases where the
+memory intervals defined by the guest aren't aligned to the host system
+page size. Is is recommended, when calling functions like mmap() and
+mprotect(), that both
+.Li addr
+and
+.Li addr + size
+be aliged to the true page size, which Blink reports to the guest in
+.Li getauxval(AT_PAGESZ).
+This value should be obtainable via the portable API
+.Li sysconf(_SC_PAGESIZE)
+assuming the C library implements it correctly. Please note that when
+Blink is running in its fully virtualized mode (i.e.
+.Li blink -m
+) this concern does not apply. That's because Blink will allocate a full
+system page for every 4096 byte page that gets mapped from a file.
 .Ss "Self Modifying Code"
 Blink supports self-modifying code, with some caveats.
 .Pp

diff --git a/blink/diself.c b/blink/diself.c
@@ -101,8 +101,6 @@ static void DisLoadElfSyms(struct Dis *d, Elf64_Ehdr_ *ehdr, size_t esize,
         d->syms.p[d->syms.i].size = Read64(st[i].size);
         unassert(d->syms.p[d->syms.i].name = strdup(stab + Read32(st[i].name)));
         d->syms.p[d->syms.i].addr = Read64(st[i].value) + eskew;
-        ELF_LOGF("SYMBOL %" PRIx64 " %" PRIx64 " %s", eskew,
-                 d->syms.p[d->syms.i].addr, d->syms.p[d->syms.i].name);
         d->syms.p[d->syms.i].rank =
             -islocal + -isweak + -isabs + isprotected + isobject + isfunc;
         d->syms.p[d->syms.i].iscode =

diff --git a/blink/instruction.c b/blink/instruction.c
@@ -21,6 +21,7 @@
 #include "blink/assert.h"
 #include "blink/bitscan.h"
 #include "blink/endian.h"
+#include "blink/linux.h"
 #include "blink/machine.h"
 #include "blink/macros.h"
 #include "blink/stats.h"
@@ -110,10 +111,9 @@ void LoadInstruction(struct Machine *m, u64 pc) {
       break;
     case kMachineSegmentationFault:
       m->faultaddr = pc;
-      ERRF("CODE CRISIS\n\t%s\n%s", GetBacktrace(m), FormatPml4t(m));
+      m->segvcode = SEGV_ACCERR_LINUX;
       HaltMachine(m, rc);
     case kMachineDecodeError:
-      ERRF("INSTRUCTION CRISIS\n\t%s", GetBacktrace(m));
       HaltMachine(m, rc);
     default:
       HaltMachine(m, rc);

diff --git a/blink/loader.c b/blink/loader.c
@@ -52,6 +52,23 @@
 
 static bool CanEmulateImpl(struct Machine *, char **, char ***, bool);
 
+static void LoaderCopy(struct Machine *m, i64 vaddr, size_t amt, void *image,
+                       i64 offset, int prot) {
+  i64 base;
+  if (!amt) return;
+  ELF_LOGF("copy %" PRIx64 "-%" PRIx64 " from %" PRIx64 "-%" PRIx64, vaddr,
+           vaddr + amt, offset, offset + amt);
+  base = ROUNDDOWN(vaddr, 4096);
+  if ((prot & (PROT_READ | PROT_WRITE)) != (PROT_READ | PROT_WRITE)) {
+    unassert(!ProtectVirtual(m->system, base, vaddr + amt - base,
+                             prot | PROT_WRITE));
+  }
+  unassert(!CopyToUser(m, vaddr, (u8 *)image + offset, amt));
+  if ((prot & (PROT_READ | PROT_WRITE)) != (PROT_READ | PROT_WRITE)) {
+    unassert(!ProtectVirtual(m->system, base, vaddr + amt - base, prot));
+  }
+}
+
 static i64 LoadElfLoadSegment(struct Machine *m, const char *path, void *image,
                               size_t imagesize, const Elf64_Phdr_ *phdr,
                               i64 last_end, u64 aslr, int fd) {
@@ -69,6 +86,9 @@ static i64 LoadElfLoadSegment(struct Machine *m, const char *path, void *image,
   u64 key = (flags & PF_R_ ? PAGE_U : 0) |   //
             (flags & PF_W_ ? PAGE_RW : 0) |  //
             (flags & PF_X_ ? 0 : PAGE_XD);
+  int prot = (flags & PF_R_ ? PROT_READ : 0) |   //
+             (flags & PF_W_ ? PROT_WRITE : 0) |  //
+             (flags & PF_X_ ? PROT_EXEC : 0);
 
   SYS_LOGF("PT_LOAD %c%c%c [%" PRIx64 ",%" PRIx64 ") %s",  //
            (flags & PF_R_ ? 'R' : '.'),                    //
@@ -150,11 +170,7 @@ static i64 LoadElfLoadSegment(struct Machine *m, const char *path, void *image,
       }
       start += pagesize;
     }
-    ELF_LOGF("copy %" PRIx64 "-%" PRIx64 " from %" PRIx64 "-%" PRIx64, vaddr,
-             vaddr + (pagesize - skew), offset,
-             offset + MIN(filesz, pagesize - skew));
-    unassert(!CopyToUser(m, vaddr, (u8 *)image + offset,
-                         MIN(filesz, pagesize - skew)));
+    LoaderCopy(m, vaddr, MIN(filesz, pagesize - skew), image, offset, prot);
     vaddr += pagesize - skew;
     offset += pagesize - skew;
     filesz -= MIN(filesz, pagesize - skew);
@@ -190,11 +206,7 @@ static i64 LoadElfLoadSegment(struct Machine *m, const char *path, void *image,
         exit(127);
       }
       // copy the tail skew.
-      if (filesz) {
-        ELF_LOGF("copy %" PRIx64 "-%" PRIx64 " from %" PRIx64 "-%" PRIx64,
-                 start, start + filesz, offset, offset + filesz);
-        unassert(!CopyToUser(m, start, (u8 *)image + offset, filesz));
-      }
+      LoaderCopy(m, start, filesz, image, offset, prot);
     } else {
       unassert(!filesz);
     }

diff --git a/blink/machine.c b/blink/machine.c
@@ -229,12 +229,7 @@ static void OpBit(P) {
     p = RegRexbRm(m, rde);
   } else {
     v = MaskAddress(Eamode(rde), ComputeAddress(A) + bitdisp);
-    p = ReserveAddress(m, v, 1 << w, false);
-    if (op == 4) {
-      SetReadAddr(m, v, 1 << w);
-    } else {
-      SetWriteAddr(m, v, 1 << w);
-    }
+    p = ReserveAddress(m, v, 1 << w, op != 4);
   }
   if (Lock(rde)) LockBus(p);
   y = 1;
@@ -368,7 +363,7 @@ static void OpMovZvqpIvqp(P) {
 }
 
 static void OpMovImm(P) {
-  WriteRegisterOrMemoryBW(rde, GetModrmReadBW(A), uimm0);
+  WriteRegisterOrMemoryBW(rde, GetModrmWriteBW(A), uimm0);
   if (IsMakingPath(m)) {
     Jitter(A,
            "a3"  // push arg3

diff --git a/blink/machine.h b/blink/machine.h
@@ -417,6 +417,7 @@ struct Machine {                           //
   bool traprdtsc;                          //
   bool trapcpuid;                          //
   i8 trapno;                               //
+  i8 segvcode;                             //
   sigjmp_buf onhalt;                       //
   struct sigaltstack_linux sigaltstack;    //
   i64 robust_list;                         //

diff --git a/blink/memory.c b/blink/memory.c
@@ -28,6 +28,7 @@
 #include "blink/endian.h"
 #include "blink/errno.h"
 #include "blink/likely.h"
+#include "blink/linux.h"
 #include "blink/log.h"
 #include "blink/machine.h"
 #include "blink/macros.h"
@@ -69,6 +70,7 @@ u64 HandlePageFault(struct Machine *m, u8 *pslot, u64 entry) {
   unassert(entry & PAGE_RSRV);
   unassert(!HasLinearMapping());
   if (m->nofault) {
+    m->segvcode = SEGV_MAPERR_LINUX;
     errno = ENOBUFS;
     return 0;
   }
@@ -88,6 +90,7 @@ u64 HandlePageFault(struct Machine *m, u8 *pslot, u64 entry) {
     } else {
       // an anonymous page is being accessed for the first time
       if ((page = AllocateAnonymousPage(m->system)) == -1) {
+        m->segvcode = SEGV_MAPERR_LINUX;
         entry = 0;
         break;
       }
@@ -198,10 +201,9 @@ u64 FindPageTableEntry(struct Machine *m, u64 page) {
     table = entry;
     index = (page >> level) & 511;
     pslot = GetPageAddress(m->system, table, level == 39) + index * 8;
+    if (!pslot) goto MapError;
     entry = LoadPte(pslot);
-    if (!(entry & PAGE_V)) {
-      return (uintptr_t)efault0();
-    }
+    if (!(entry & PAGE_V)) goto MapError;
     if (m->metal) {
       entry &= ~(u64)(PAGE_RSRV | PAGE_HOST | PAGE_MAP | PAGE_GROW | PAGE_MUG |
                       PAGE_FILE);
@@ -231,20 +233,25 @@ u64 FindPageTableEntry(struct Machine *m, u64 page) {
           entry += PAGE_LOCK;
         } else {
           ReleasePageLock(pslot);
+          m->segvcode = SEGV_MAPERR_LINUX;
           return 0;
         }
       } else {
         goto TryAgain;
       }
     } else {
       LOGF("too many threads locked page %#" PRIx64, page);
+      m->segvcode = SEGV_MAPERR_LINUX;
       errno = EAGAIN;
       return 0;
     }
   }
   m->tlb[ARRAYLEN(m->tlb) - 1].page = page;
   m->tlb[ARRAYLEN(m->tlb) - 1].entry = entry;
   return entry;
+MapError:
+  m->segvcode = SEGV_MAPERR_LINUX;
+  return (uintptr_t)efault0();
 }
 
 u8 *LookupAddress2(struct Machine *m, i64 virt, u64 mask, u64 need) {
@@ -264,21 +271,25 @@ u8 *LookupAddress2(struct Machine *m, i64 virt, u64 mask, u64 need) {
         return 0;
       }
     } else {
+      m->segvcode = SEGV_MAPERR_LINUX;
       return (u8 *)efault0();
     }
   } else if (virt >= 0 && virt <= 0xffffffff &&
              (virt & 0xffffffff) + 4095 < kRealSize) {
     unassert(m->system->real);
     return m->system->real + virt;
   } else {
+    m->segvcode = SEGV_MAPERR_LINUX;
     return (u8 *)efault0();
   }
   if ((entry & mask) != need) {
+    m->segvcode = SEGV_ACCERR_LINUX;
     return (u8 *)efault0();
   }
   if ((host = GetPageAddress(m->system, entry, false))) {
     return host + (virt & 4095);
   } else {
+    m->segvcode = SEGV_MAPERR_LINUX;
     return (u8 *)efault0();
   }
 }
@@ -381,18 +392,57 @@ void CommitStash(struct Machine *m) {
 }
 
 u8 *ReserveAddress(struct Machine *m, i64 v, size_t n, bool writable) {
-  if (HasLinearMapping()) return ToHost(v);
+  long k;
+  u64 mask, need;
+  u8 *res, *p1, *p2;
+  if (writable) {
+    SetWriteAddr(m, v, n);
+  } else {
+    SetReadAddr(m, v, n);
+  }
+  if (HasLinearMapping()) {
+    return ToHost(v);
+  }
   m->reserving = true;
+  if (Cpl(m) == 3) {
+    if (!writable) {
+      mask = PAGE_U;
+      need = PAGE_U;
+    } else {
+      mask = PAGE_U | PAGE_RW;
+      need = PAGE_U | PAGE_RW;
+    }
+  } else {
+    mask = 0;
+    need = 0;
+  }
   if ((v & 4095) + n <= 4096) {
-    return ResolveAddress(m, v);
+    if ((res = LookupAddress2(m, v, mask, need))) {
+      return res;
+    } else {
+      ThrowSegmentationFault(m, v);
+    }
   }
   STATISTIC(++page_overlaps);
+  unassert(n <= 4096);
   m->stashaddr = v;
   m->opcache->stashsize = n;
   m->opcache->writable = writable;
-  u8 *r = m->opcache->stash;
-  CopyFromUser(m, r, v, n);
-  return r;
+  res = m->opcache->stash;
+  k = 4096 - (v & 4095);
+  if ((p1 = LookupAddress2(m, v, mask, need))) {
+    if ((p2 = LookupAddress2(m, v + k, mask, need))) {
+      IGNORE_RACES_START();
+      memcpy(res, p1, k);
+      memcpy(res + k, p2, n - k);
+      IGNORE_RACES_END();
+      return res;
+    } else {
+      ThrowSegmentationFault(m, v + k);
+    }
+  } else {
+    ThrowSegmentationFault(m, v);
+  }
 }
 
 u8 *AccessRam(struct Machine *m, i64 v, size_t n, void *p[2], u8 *tmp,

diff --git a/blink/memorymalloc.c b/blink/memorymalloc.c
@@ -825,8 +825,10 @@ i64 ReserveVirtual(struct System *s, i64 virt, i64 size, u64 flags, int fd,
     }
   }
 
-  prot = ((flags & PAGE_U ? PROT_READ : 0) |
-          ((flags & PAGE_RW) || fd == -1 ? PROT_WRITE : 0));
+  // determine host memory protection
+  prot = 0;
+  if (flags & PAGE_U) prot |= PROT_READ;
+  if (flags & PAGE_RW) prot |= PROT_WRITE;
 
   if (HasLinearMapping()) {
     // create a linear mapping. doing this runs the risk of destroying

diff --git a/blink/modrm.c b/blink/modrm.c
@@ -105,10 +105,7 @@ i64 ComputeAddress(P) {
 }
 
 u8 *ComputeReserveAddressRead(P, size_t n) {
-  i64 v;
-  v = ComputeAddress(A);
-  SetReadAddr(m, v, n);
-  return ReserveAddress(m, v, n, false);
+  return ReserveAddress(m, ComputeAddress(A), n, false);
 }
 
 u8 *ComputeReserveAddressRead1(P) {
@@ -124,10 +121,7 @@ u8 *ComputeReserveAddressRead8(P) {
 }
 
 u8 *ComputeReserveAddressWrite(P, size_t n) {
-  i64 v;
-  v = ComputeAddress(A);
-  SetWriteAddr(m, v, n);
-  return ReserveAddress(m, v, n, true);
+  return ReserveAddress(m, ComputeAddress(A), n, true);
 }
 
 u8 *ComputeReserveAddressWrite1(P) {

diff --git a/blink/throw.c b/blink/throw.c
@@ -82,11 +82,13 @@ void HaltMachine(struct Machine *m, int code) {
       break;
     case kMachineProtectionFault:
       RestoreIp(m);
-      DeliverSignalToUser(m, SIGSEGV_LINUX, SEGV_ACCERR_LINUX);
+      m->faultaddr = m->ip;
+      DeliverSignalToUser(m, SIGILL_LINUX, ILL_PRVOPC_LINUX);
       break;
     case kMachineSegmentationFault:
       RestoreIp(m);
-      DeliverSignalToUser(m, SIGSEGV_LINUX, SEGV_MAPERR_LINUX);
+      DeliverSignalToUser(m, SIGSEGV_LINUX,
+                          m->segvcode ? m->segvcode : SEGV_MAPERR_LINUX);
       break;
     case 1:
     case 3:

diff --git a/test/asm/asm.mk b/test/asm/asm.mk
@@ -46,6 +46,10 @@ o/$(MODE)/test/asm/%.com:						\
 	@echo "$(VM) $< || exit" >>$@
 	@echo "echo [test] o/$(MODE)/blink/blink $< >&2" >>$@
 	@echo "o/$(MODE)/blink/blink $< || exit" >>$@
+	@echo "echo [test] o/$(MODE)/blink/blink -m $< >&2" >>$@
+	@echo "o/$(MODE)/blink/blink -m $< || exit" >>$@
+	@echo "echo [test] o/$(MODE)/blink/blink -jm $< >&2" >>$@
+	@echo "o/$(MODE)/blink/blink -jm $< || exit" >>$@
 	@echo "echo [test] o/third_party/qemu/qemu-x86_64 -cpu core2duo $< >&2" >>$@
 	@echo "$(VM) o/third_party/qemu/qemu-x86_64 -cpu core2duo $< || exit" >>$@
 	@chmod +x $@