refactor: improve clarity regarding some NetBSD quirks

Author: darfink

src/alloc.rs

@@ -190,7 +190,21 @@ mod tests {
 
   #[test]
   fn alloc_frees_memory_when_dropped() -> Result<()> {
-    let base = alloc(1, Protection::READ_WRITE)?.as_ptr::<()>();
+    // Designing these tests can be quite tricky sometimes. When a page is
+    // allocated and then released, a subsequent `query` may allocate memory in
+    // the same location that has just been freed. For instance, NetBSD's
+    // kinfo_getvmmap uses `mmap` internally, which can lead to potentially
+    // confusing outcomes. To mitigate this, an additional buffer region is
+    // allocated to ensure that any memory allocated indirectly through `query`
+    // occupies a separate location in memory.
+    let (start, _buffer) = (
+      alloc(1, Protection::READ_WRITE)?,
+      alloc(1, Protection::READ_WRITE)?,
+    );
+
+    let base = start.as_ptr::<()>();
+    std::mem::drop(start);
+
     let query = crate::query(base);
     assert!(matches!(query, Err(Error::UnmappedRegion)));
     Ok(())
@@ -205,7 +219,7 @@ mod tests {
   }
 
   #[test]
-  #[cfg(not(target_os = "openbsd"))]
+  #[cfg(not(any(target_os = "openbsd", target_os = "netbsd")))]
   fn alloc_can_allocate_executable_region() -> Result<()> {
     let memory = alloc(1, Protection::WRITE_EXECUTE)?;
     assert_eq!(memory.len(), page::size());

src/os/openbsd.rs

@@ -30,7 +30,7 @@ impl Iterator for QueryIter {
   fn next(&mut self) -> Option<Self::Item> {
     let mut len = std::mem::size_of::<kinfo_vmentry>();
 
-    // Albeit it would be preferred to query the information for all virtual
+    // Although it would be preferred to query the information for all virtual
     // pages at once, the system call does not seem to respond consistently. If
     // called once during a process' lifetime, it returns all pages, but if
     // called again, it returns an empty buffer. This may be caused due to an

src/os/unix.rs

@@ -8,12 +8,16 @@ pub fn page_size() -> usize {
 }
 
 pub unsafe fn alloc(base: *const (), size: usize, protection: Protection) -> Result<*const ()> {
-  #[cfg(not(target_os = "netbsd"))]
-  let prot = protection.to_native();
+  let mut prot = protection.to_native();
+
+  // This adjustment ensures that the behavior of memory protection is
+  // orthogonal across all platforms by aligning NetBSD's protection flags
+  // and PaX behavior with those of other operating systems.
+  if cfg!(target_os = "netbsd") {
+    let max_protection = (PROT_READ | PROT_WRITE | PROT_EXEC) << 3;
+    prot |= max_protection;
+  }
 
-  // PROT_MPROTECT usage for avoiding problems with NetBSD pax
-  #[cfg(target_os = "netbsd")]
-  let prot = protection.to_native() | (PROT_READ | PROT_WRITE | PROT_EXEC) << 3;
   let mut flags = MAP_PRIVATE | MAP_ANON;
 
   if !base.is_null() {

src/protect.rs

@@ -170,6 +170,7 @@ mod tests {
   #[test]
   #[cfg(not(any(
     target_os = "openbsd",
+    target_os = "netbsd",
     all(target_vendor = "apple", target_arch = "aarch64")
   )))]
   fn protect_can_alter_text_segments() {

src/query.rs

@@ -37,12 +37,12 @@ impl Iterator for QueryIter {
         Ok(region) => {
           let range = region.as_range();
 
-          // Skip the region if it is prior to the queried range
+          // Skip the region if it precedes the queried range
           if range.end <= self.origin as usize {
             continue;
           }
 
-          // Stop iteration if the region is after the queried range
+          // Stop iteration if the region is past the queried range
           if range.start >= regions.upper_bound() {
             break;
           }