Merge pull request #202 from urbit/msl/guard

guard page and `bail:meme`

rust/ares/Cargo.lock

@@ -62,7 +62,7 @@ dependencies = [
  "ares_guard",
  "ares_macros",
  "ares_pma",
- "assert_no_alloc 1.1.2",
+ "assert_no_alloc",
  "autotools",
  "bitvec",
  "cc",
@@ -87,7 +87,7 @@ version = "0.1.0"
 dependencies = [
  "aes",
  "aes-siv",
- "assert_no_alloc 1.1.2 (registry+https://github.com/rust-lang/crates.io-index)",
+ "assert_no_alloc",
  "curve25519-dalek",
  "ed25519-dalek",
  "ibig 0.3.6 (registry+https://github.com/rust-lang/crates.io-index)",
@@ -124,12 +124,6 @@ dependencies = [
 name = "assert_no_alloc"
 version = "1.1.2"
 
-[[package]]
-name = "assert_no_alloc"
-version = "1.1.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "55ca83137a482d61d916ceb1eba52a684f98004f18e0cafea230fe5579c178a3"
-
 [[package]]
 name = "atty"
 version = "0.2.14"

rust/ares/Cargo.toml

@@ -18,8 +18,8 @@ ares_macros = { path = "../ares_macros" }
 # ares_pma = { path = "../ares_pma", features=["debug_prints"] }
 ares_pma = { path = "../ares_pma" }
 # use this when debugging requires allocation (e.g. eprintln)
-# assert_no_alloc = { path = "../assert-no-alloc", features=["warn_debug"] }
-assert_no_alloc = { path = "../assert-no-alloc" }
+# assert_no_alloc = { path = "../rust-assert-no-alloc", features=["warn_debug"] }
+assert_no_alloc = { path = "../rust-assert-no-alloc" }
 bitvec = "1.0.0"
 criterion = "0.4"
 either = "1.9.0"

rust/ares/src/serf.rs

@@ -1,4 +1,3 @@
-use crate::guard::call_with_guard;
 use crate::hamt::Hamt;
 use crate::interpreter;
 use crate::interpreter::{inc, interpret, Error, Mote};
@@ -403,27 +402,7 @@ fn slam(context: &mut Context, axis: u64, ovo: Noun) -> Result<Noun, Error> {
     let fol = T(stack, &[D(8), pul, D(9), D(2), D(10), sam, D(0), D(2)]);
     let sub = T(stack, &[arvo, ovo]);
 
-    let frame_p = stack.get_frame_pointer();
-    let stack_pp = stack.get_stack_pointer_pointer();
-    let alloc_pp = stack.get_alloc_pointer_pointer();
-
-    let res = call_with_guard(
-        stack_pp as *const *const u64,
-        alloc_pp as *const *const u64,
-        &mut || interpret(&mut context.nock_context, sub, fol),
-    );
-
-    if let Err(Error::NonDeterministic(Mote::Meme, _)) = res {
-        unsafe {
-            let stack = &mut context.nock_context.stack;
-            assert_no_alloc::reset_counters();
-            while stack.get_frame_pointer() != frame_p {
-                stack.frame_pop();
-            }
-        }
-    }
-
-    res
+    interpret(&mut context.nock_context, sub, fol)
 }
 
 fn peek(context: &mut Context, ovo: Noun) -> Noun {
@@ -626,7 +605,7 @@ fn work_swap(context: &mut Context, job: Noun, goof: Noun) {
             context.work_swap(ovo, fec);
         }
         Err(goof_crud) => {
-            eprintln!("\r serf: bail");
+            eprintln!("\rserf: bail");
             let stack = &mut context.nock_context.stack;
             let lud = T(stack, &[goof_crud, goof, D(0)]);
             context.work_bail(lud);

rust/ares_crypto/Cargo.toml

@@ -7,8 +7,8 @@ edition = "2021"
 
 [dependencies]
 # use this when debugging requires allocation (e.g. eprintln)
-# assert_no_alloc = {version="1.1.2", features=["warn_debug"]}
-assert_no_alloc = "1.1.2"
+# assert_no_alloc = { path = "../rust-assert-no-alloc", features=["warn_debug"] }
+assert_no_alloc = { path = "../rust-assert-no-alloc" }
 ibig = "0.3.6"
 
 # ed25519

rust/ares_guard/c-src/guard.c

@@ -22,54 +22,56 @@
 #endif
 
 /**
- *  XX: documentation
+ * Linked list stack of jump buffers.
  */
-typedef struct _GD_state GD_state;
-struct _GD_state {
+typedef struct GD_buflistnode GD_buflistnode;
+struct GD_buflistnode {
+  jmp_buf         buffer;
+  GD_buflistnode *next;
+};
+
+/**
+ * Global guard page state.
+ */
+typedef struct GD_state GD_state;
+struct GD_state {
   uintptr_t         guard_p;
   const uintptr_t  *stack_pp;
   const uintptr_t  *alloc_pp;
-  jmp_buf           env_buffer;
+  GD_buflistnode   *buffer_list;
   struct sigaction  prev_sa;
 };
 
-static GD_state *_guard_state = NULL;
-
+static GD_state _gd_state = { 
+  .guard_p = 0,
+  .stack_pp = NULL,
+  .alloc_pp = NULL,
+  .buffer_list = NULL,
+  .prev_sa = { .sa_sigaction = NULL, .sa_flags = 0 },
+};
 
 static uint32_t
-_prot_page(void *address, int prot)
+_protect_page(void *address, int prot)
 {
   if (mprotect(address, GD_PAGE_SIZE, prot)) {
     fprintf(stderr, "guard: prot: mprotect error %d\r\n", errno);
     fprintf(stderr, "%s\r\n", strerror(errno));
-    return guard_mprotect | errno;
+    return guard_mprotect ;
   }
 
   return 0;
 }
 
+// Center the guard page.
 static uint32_t
-_mark_page(void *address)
+_focus_guard(GD_state *gd)
 {
-  return _prot_page(address, PROT_NONE);
-}
+  uintptr_t stack_p = *(gd->stack_pp);
+  uintptr_t alloc_p = *(gd->alloc_pp);
+  uintptr_t old_guard_p = (gd->guard_p);
+  uintptr_t new_guard_p;
+  uint32_t  err = 0;
 
-static uint32_t
-_unmark_page(void *address)
-{
-  return _prot_page(address, PROT_READ | PROT_WRITE);
-}
-
-/**
- * Center the guard page.
- */
-static uint32_t
-_focus_guard(GD_state *gs) {
-  uintptr_t  *guard_pp = &(gs->guard_p);
-  uintptr_t   stack_p = *(gs->stack_pp);
-  uintptr_t   alloc_p = *(gs->alloc_pp);
-
-  // Check anomalous arguments
   if (stack_p == 0 || alloc_p == 0) {
     fprintf(stderr, "guard: focus: stack or alloc pointer is null\r\n");
     return guard_null;
@@ -77,89 +79,81 @@ _focus_guard(GD_state *gs) {
     return guard_oom;
   }
 
-  uintptr_t old_guard_p = *guard_pp;
-  uintptr_t new_guard_p;
-  int32_t   err = 0;
-
-  // Compute new guard page
-  // XX:  Should we also check for new_guard_p < min(stack_p, alloc_p)?
+  // Compute new guard page.
   new_guard_p = GD_PAGE_ROUND_DOWN((stack_p + alloc_p) / 2);
   if (new_guard_p == old_guard_p) {
     return guard_oom;
   }
 
-  // Mark new guard page
-  if ((err = _mark_page((void *)new_guard_p))) {
-    fprintf(stderr, "guard: focus: mark error %p\r\n", (void *)new_guard_p);
+  // Mark new guard page.
+  if ((err = _protect_page((void *)new_guard_p, PROT_NONE))) {
+    fprintf(stderr, "guard: focus: mark error\r\n");
     return err;
   }
 
-  // Update guard page tracker
-  *guard_pp = new_guard_p;
+  // Update guard page tracker.
+  gd->guard_p = new_guard_p;
 
-  // Unmark the old guard page (if there is one)
+  // Unmark the old guard page if there is one.
   if (old_guard_p) {
-    if ((err = _unmark_page((void *)old_guard_p))) {
-      fprintf(stderr, "guard: focus: unmark error, %p\r\n", (void *)old_guard_p);
+    if ((err = _protect_page((void *)old_guard_p, PROT_READ | PROT_WRITE))) {
+      fprintf(stderr, "guard: focus: unmark error\r\n");
       return err;
     }
   }
-  
+
   return 0;
 }
 
-void
+static void
 _signal_handler(int sig, siginfo_t *si, void *unused)
 {
   uintptr_t sig_addr;
-  int32_t   err = 0;
+  uint32_t err = 0;
 
-  assert(sig == GD_SIGNAL);
+  assert(_gd_state.guard_p);
+  if (sig != GD_SIGNAL) {
+    fprintf(stderr, "guard: handler: invalid signal: %d\r\n", sig);
+    assert(0);
+  }
 
   sig_addr = (uintptr_t)si->si_addr;
-  if (
-    sig_addr >= _guard_state->guard_p && 
-    sig_addr <  (_guard_state->guard_p + GD_PAGE_SIZE))
+
+  if (sig_addr >= _gd_state.guard_p &&
+      sig_addr <  _gd_state.guard_p + GD_PAGE_SIZE)
   {
-    err = _focus_guard(_guard_state);
+    err = _focus_guard(&_gd_state);
     if (err) {
-      siglongjmp(_guard_state->env_buffer, err);
+      siglongjmp(_gd_state.buffer_list->buffer, err);
     }
-  } else {
-    struct sigaction prev_sa = _guard_state->prev_sa;
+  }
+  else {
+    struct sigaction prev_sa = _gd_state.prev_sa;
 
     if (prev_sa.sa_sigaction != NULL) {
       prev_sa.sa_sigaction(sig, si, unused);
     } else if (prev_sa.sa_handler != NULL) {
       prev_sa.sa_handler(sig);
     } else {
-      // There should always be a default SIGSEGV handler
+      // There should always be a default handler
       assert(0);
     }
   }
 }
 
-uint32_t
-_register_handler(GD_state *gs)
+// Registers the handler function.
+static uint32_t
+_register_handler(GD_state *gd)
 {
   struct sigaction sa;
 
-  // Flag to use sa_sigaction
   sa.sa_flags = SA_SIGINFO;
-  // Must use sa_sigaction; sa-handler takes signal handler as its only argument
   sa.sa_sigaction = _signal_handler;
-  // Set mask of signals to ignore while running signal handler
-  // XX:  By default the signal that triggered the signal handler is automatically added to the
-  //      mask while it's being handled, so unless we plan to add more signals to this then I don't
-  //      think it's necessary.
-  // sigemptyset(&sa.sa_mask);
-  // sigaddset(&(sa.sa_mask), SIGSEGV);
 
-  // Set the new SIGSEGV handler, and save the old SIGSEGV handler (if any)
-  if (sigaction(GD_SIGNAL, &sa, &(gs->prev_sa))) {
+  if (sigaction(GD_SIGNAL, &sa, &(gd->prev_sa))) {
     fprintf(stderr, "guard: register: sigaction error\r\n");
     fprintf(stderr, "%s\r\n", strerror(errno));
-    return guard_sigaction | errno;
+    return guard_sigaction;
   }
 
   return 0;
@@ -167,66 +161,83 @@ _register_handler(GD_state *gs)
 
 uint32_t
 guard(
-  callback f,
+  void *(*f)(void *),
   void *closure,
-  const uintptr_t *const stack_pp,
-  const uintptr_t *const alloc_pp,
-  void ** ret
+  const uintptr_t *const s_pp,
+  const uintptr_t *const a_pp,
+  void **ret
 ) {
-  int32_t err = 0;
-  int32_t err_c = 0;
+  GD_buflistnode *new_buffer;
+  uint32_t        err = 0;
+  uint32_t        td_err = 0;
 
-  //
-  // Setup guard page state
-  //
+  if (_gd_state.guard_p == 0) {
+    assert(_gd_state.buffer_list == NULL);
 
-  // guard() presumes that it is only ever called once at a time
-  assert(_guard_state == NULL);
+    _gd_state.stack_pp = s_pp;
+    _gd_state.alloc_pp = a_pp;
 
-  _guard_state = (GD_state *)malloc(sizeof(GD_state));
-  if (_guard_state == NULL) {
+    // Initialize the guard page.
+    if ((err = _focus_guard(&_gd_state))) {
+      fprintf(stderr, "guard: initial focus error\r\n");
+      goto exit;
+    }
+
+    // Register guard page signal handler.
+    if ((err = _register_handler(&_gd_state))) {
+      fprintf(stderr, "guard: registration error\r\n");
+      goto tidy;
+    }
+  } else {
+    assert(_gd_state.buffer_list != NULL);
+  }
+
+  // Setup new longjmp buffer.
+  new_buffer = (GD_buflistnode *)malloc(sizeof(GD_buflistnode));
+  if (new_buffer == NULL) {
     fprintf(stderr, "guard: malloc error\r\n");
     fprintf(stderr, "%s\r\n", strerror(errno));
-    return guard_malloc | errno;
+    err = guard_malloc;
+    goto skip;
   }
-  _guard_state->guard_p = 0;
-  _guard_state->stack_pp = stack_pp;
-  _guard_state->alloc_pp = alloc_pp;
+  new_buffer->next = _gd_state.buffer_list;
+  _gd_state.buffer_list = new_buffer;
 
-  // Initialize the guard page
-  if ((err = _focus_guard(_guard_state))) {
-    goto clear;
-  }
-
-  // Register guard page signal handler
-  if ((err = _register_handler(_guard_state))) {
-    goto unmark;
-  }
-
-  //
-  // Run closure
-  //
-
-  if (!(err = sigsetjmp(_guard_state->env_buffer, 1))) {
+  // Run given closure.
+  if (!(err = sigsetjmp(_gd_state.buffer_list->buffer, 1))) {
     *ret = f(closure);
   }
 
-  //
-  // Clean up guard page state
-  //
+  // Restore previous longjmp buffer.
+  _gd_state.buffer_list = _gd_state.buffer_list->next;
+  free((void *)new_buffer);
 
-  if (sigaction(GD_SIGNAL, &(_guard_state->prev_sa), NULL)) {
-    fprintf(stderr, "guard: sigaction error\r\n");
-    fprintf(stderr, "%s\r\n", strerror(errno));
-    err_c = guard_sigaction | errno;
+skip:
+  if (_gd_state.buffer_list == NULL) {
+    if (sigaction(GD_SIGNAL, &_gd_state.prev_sa, NULL)) {
+      fprintf(stderr, "guard: error replacing sigsegv handler\r\n");
+      fprintf(stderr, "%s\r\n", strerror(errno));
+      td_err = guard_sigaction;
+
+      if (!err) {
+        err = td_err;
+      }
+    }
+
+tidy:
+    // Unmark guard page.
+    assert(_gd_state.guard_p != 0);
+    td_err = _protect_page((void *)_gd_state.guard_p, PROT_READ | PROT_WRITE);
+    if (td_err) {
+      fprintf(stderr, "guard: unmark error\r\n");
+      fprintf(stderr, "%s\r\n", strerror(errno));
+      if (!err) {
+        err = td_err;
+      }
+    }
+    _gd_state.guard_p = 0;
   }
 
-unmark:
-  err_c = _unmark_page((void *)_guard_state->guard_p);
-
-clear:
-  free(_guard_state);
-  _guard_state = NULL;
-
-  return err ? err : err_c;
+exit:
+  return err;
 }

rust/ares_guard/c-src/guard.h

@@ -1,58 +1,73 @@
 #ifndef __GUARD_H__
 #define __GUARD_H__
 
+#include <setjmp.h>
 #include <stdint.h>
 
 /**
- * Error codes and flags.
- * 
- * The flags are bitwise added to the errno of their respective errors.
+ * Error codes.
  */
 typedef enum {
-  guard_null      = 1,          // null stack or alloc pointer
-  guard_oom,                    // OOM
-  guard_malloc    = 0x10000000, // malloc error flag
-  guard_mprotect  = 0x20000000, // mprotect error flag
-  guard_sigaction = 0x40000000, // sigaction error flag
+  guard_null,       // null stack or alloc pointer
+  guard_signal,     // invalid signal
+  guard_oom,        // out of memory
+  guard_malloc,     // malloc error
+  guard_mprotect,   // mprotect error
+  guard_sigaction,  // sigaction error
 } guard_err;
 
-typedef void *(*callback)(void *);
-
 /**
- * Execute the given closure `f` within the memory arena between the
- * `stack` and `alloc` pointers, with guard page protection. Write either
- * `f`'s succesful result or a `guard_err` to the given `ret` pointer.
+ * @brief Executes the given callback function `f` within the memory arena 
+ * between the stack and allocation pointers pointed to by `s_pp` and `a_pp`,
+ * with guard page protection. If `f`'s execution succeeds, its result is 
+ * written to the return pointer `*ret`. If `f`'s execution triggers an
+ * out of memory error or any other `guard_err`, the `guard_err` is
+ * returned and `*ret` is left empty. In either case, cleanup is performed
+ * before returning.
+ * 
+ * Definitions:
+ * - A guard page is marked `PROT_NONE`.
  *
- * Memory
- * ------
- * The free memory arena between the `stack` and `alloc` pointers is part of a
- * NockStack frame, which may either face east or west. If the frame faces
- * east, the `stack` pointer will be greater than the `alloc` pointer. If it
- * faces west, the `stack` pointer will be less than the `alloc` pointer.
+ * Assumptions:
+ * - `NockStack` pages are marked `PROT_READ|PROT_WRITE` by default.
+ * - All memory access patterns are outside-in.
+ * - Callback functions are compatible with the C ABI.
+ * - `NockStack` stack and allocation pointer locations are fixed.
+ * - The caller is responsible for return value memory allocation.
+ * - The caller is responsible for managing any external state the callback
+ *   function may mutate.
+ * - The callback function may be interrupted in the case of memory exhaustion
+ *   or other `guard_err` error (failure to `mprotect`, `malloc`, etc.).
+ * - `SIGSEGV` (`SIGBUS` on macOS) signals are expected to be raised only on
+ *    guard page accesses.
  *
- * All the pages in the memory arena are marked clean (`PROT_READ | PROT_WRITE`)
- * by default, with the exception of a single guard page in the middle of the
- * arena, which is marked with `PROT_NONE`.
+ * Invariants:
+ * - A single guard page is installed and maintained in the approximate center
+ *   until `crate::guard::call_with_guard` returns.
+ * - A return value is only written to `*ret` on successful callback execution.
+ * - A `guard_err` is returned.
  *
- * Guard
- * -----
- * This function protects the free memory arena between the `stack` and `alloc`
- * pointers with a guard page. A guard page is simply a single page of memory
- * which is marked with `PROT_NONE`. Since all other pages are marked clean by
- * default, a SIGSEGV will only be raised if the `f` function attempts to write
- * to the guard page. When it does, the signal handler will attempt to re-center
- * the guard page in the remaining free space left in the arena. If there is no
- * more free space, then memory exhaustion has occurred and the `guard_spent`
- * error will be written to the `ret` pointer. The caller is then responsible
- * for handling this error and aborting with a `bail:meme`.
+ * Enhancements:
+ * - Use only a single, static jump buffer variable instead of a linked list.
+ *   We currently use a linked list of jump buffers because we don't have a
+ *   function for preserving stack traces across `crate::interpreter::interpret`
+ *   calls.
+ *
+ * @param f The callback function to execute.
+ * @param closure A pointer to the closure data for the callback function.
+ * @param s_pp A pointer to the stack pointer location.
+ * @param a_pp A pointer to the allocation pointer location.
+ * @param ret A pointer to a location where the callback's result can be stored.
+ * 
+ * @return 0 on callback success; otherwise `guard_err` error code.
  */
 uint32_t
 guard(
-  callback f,
+  void *(*f)(void *),
   void *closure,
-  const uintptr_t *const stack_pp,
-  const uintptr_t *const alloc_pp,
-  void ** ret
+  const uintptr_t *const s_pp,
+  const uintptr_t *const a_pp,
+  void **ret
 );
 
 #endif  // __GUARD_H__

rust/ares_guard/src/lib.rs

@@ -4,8 +4,9 @@
 
 include!(concat!(env!("OUT_DIR"), "/bindings.rs"));
 
-pub const GUARD_NULL:       u32 = guard_err_guard_null; 
-pub const GUARD_OOM:        u32 = guard_err_guard_oom;
-pub const GUARD_MALLOC:     u32 = guard_err_guard_malloc;
-pub const GUARD_MPROTECT:   u32 = guard_err_guard_mprotect;
-pub const GUARD_SIGACTION:  u32 = guard_err_guard_sigaction;
+pub const GUARD_NULL:      u32 = guard_err_guard_null;
+pub const GUARD_SIGNAL:    u32 = guard_err_guard_signal;
+pub const GUARD_OOM:       u32 = guard_err_guard_oom;
+pub const GUARD_MALLOC:    u32 = guard_err_guard_malloc;
+pub const GUARD_MPROTECT:  u32 = guard_err_guard_mprotect;
+pub const GUARD_SIGACTION: u32 = guard_err_guard_sigaction;

rust/ares_pma/c-src/btree.c

@@ -76,6 +76,7 @@ STATIC_ASSERT(0, "debugger break instruction unimplemented");
 /* the opposite of P2BYTES */
 #define B2PAGES(x) ((size_t)(x) >> BT_PAGEBITS)
 
+
 #define __packed        __attribute__((__packed__))
 #define UNUSED(x) ((void)(x))
 
@@ -94,6 +95,7 @@ STATIC_ASSERT(0, "debugger break instruction unimplemented");
 /* given a pointer p returns the low page-aligned addr */
 #define LO_ALIGN_PAGE(p) ((BT_page *)(((uintptr_t)p) & ~(BT_PAGESIZE - 1)))
 
+
 #define BT_MAPADDR  ((BYTE *) S(0x1000,0000,0000))
 
 static inline vaof_t
@@ -339,6 +341,7 @@ struct BT_state {
 
 /*
 
+
 //// ===========================================================================
 ////                            btree internal routines
 

rust/rust-assert-no-alloc/Cargo.toml

@@ -12,6 +12,7 @@ keywords = ["allocator", "real-time", "debug", "audio"]
 categories = ["development-tools::debugging"]
 
 [features]
+default = []
 warn_debug = []
 warn_release = []
 disable_release = []

rust/rust-assert-no-alloc/src/lib.rs

@@ -91,6 +91,22 @@ pub fn assert_no_alloc<T, F: FnOnce() -> T> (func: F) -> T {
 	return ret;
 }
 
+/// Calls the `func` closure, but ensures that the forbid and permit counters
+/// are maintained accurately even if a longjmp originates and terminates
+/// within the closure. If you longjmp over this function, we can't fix
+/// anything about it.
+pub fn ensure_alloc_counters<T, F: FnOnce() -> T> (func: F) -> T {
+	let forbid_counter = ALLOC_FORBID_COUNT.with(|c| c.get());
+	let permit_counter = ALLOC_PERMIT_COUNT.with(|c| c.get());
+
+	let ret = func();
+
+	ALLOC_FORBID_COUNT.with(|c| c.set(forbid_counter));
+	ALLOC_PERMIT_COUNT.with(|c| c.set(permit_counter));
+
+	return ret;
+}
+
 #[cfg(not(all(feature = "disable_release", not(debug_assertions))))] // if not disabled
 /// Calls the `func` closure. Allocations are temporarily allowed, even if this
 /// code runs inside of assert_no_alloc.
@@ -132,13 +148,6 @@ pub fn reset_violation_count() {
 	ALLOC_VIOLATION_COUNT.with(|c| c.set(0));
 }
 
-pub fn reset_counters() {
-	ALLOC_FORBID_COUNT.with(|c| c.set(0));
-	ALLOC_PERMIT_COUNT.with(|c| c.set(0));
-
-	#[cfg(any( all(feature="warn_debug", debug_assertions), all(feature="warn_release", not(debug_assertions)) ))]
-	ALLOC_VIOLATION_COUNT.with(|c| c.set(0));
-}