Simple mmap allocator compulsively segfaults

Hello people of the Rust "ecosystem".

While I am keen on using the language, I have a lot of trouble going through the never-ending mess that happens between me writing something which looks vaguely understandable and disambiguous, using my wondrous "Hello World !"-worthy skills, and whatever output rustc and llvm defecate, with a magnificent gazillion calls of startup functions and closure which inevitably pollutes my core dumps when the unavoidable segfault happens as I run my garbage.

I am left asking your help following a very bitter incident : I wrote an extremely simple memory allocator which, according to my needs, simply mmaps every individual allocation, and frees them up all at once. Reasons for using mmap directly is that said allocations are expected to be FAT, not too numerous and temporary, which makes directly mmaping anonymous pages the easy way (malloc would have done the exact same with those sizes, except with a lot more overhead). As expected with this brilliantly unsafe code, it promptly became a segfault industrial machine, which I have spent the past week trying to desperately understand. Not only does it constantly segfault at whimsical moments (it is currently stuck at faulting when trying to populate an allocated slice of i64s, of length 481), but it also has interesting memusage and valgrind reports, with huge stack and heap (malloc) usage despite absolutely nothing being written with the intent of doing so.

To add to the humiliation, I have rewritten that code in C this afternoon, in about 20 minutes, having no prior experience ever in writing C, just to see whether I just sucked at basic programming, or if there was some other problem. Well, the C version works both in debug config and with gcc -O3. And both memusage and valgrind state clear as rock that stack and heap are both unused, even during the test (which is strange, because there are a few pointers I'd expect to live on the stack).

Enough words, here is the rust implementation:

use std::num::NonZero;

pub struct TmpReentrantHdr {
	size: usize,
	next: *mut TmpReentrantHdr
}

pub unsafe fn tmpmap<T> (
	markp: *mut *mut TmpReentrantHdr,
	rsize: usize
) -> *mut T {
	let tsize: NonZero<usize> = unsafe {
		NonZero::new_unchecked(
			(rsize + size_of::<TmpReentrantHdr>())+(8-rsize%8)%8
		)
	};
	use nix::sys::mman::{
		ProtFlags,
		MapFlags,
		mmap_anonymous
	};
	use nix::libc::{
		PROT_READ,
		PROT_WRITE,
		MAP_PRIVATE
	};
	let pflags: ProtFlags =
		ProtFlags::from_bits_retain(
			PROT_READ | PROT_WRITE
		);
	let mflags: MapFlags =
		MapFlags::from_bits_retain(
			MAP_PRIVATE
		);
	let bptr: *mut u8 = unsafe {
		mmap_anonymous(None, tsize, pflags, mflags)
			.expect("mmap failure with error")
			.cast::<u8>()
			.as_ptr()
	};
	let hptr: *mut TmpReentrantHdr =
		bptr.cast::<TmpReentrantHdr>();
	assert!(hptr.is_aligned());
	unsafe {
		*hptr = TmpReentrantHdr {
			size: tsize.get(),
			next: *markp
		}
	};
	unsafe { 
		*markp = hptr
	};
	unsafe {
	assert!(tsize.get() == (**markp).size)
	};
	let rptr: *mut T = unsafe {
		hptr
			.add(size_of::<TmpReentrantHdr>())
			.cast::<T>()
	};
	assert!(rptr.is_aligned());
	rptr
}

pub unsafe fn tmpmapfree(
	hdrp: *mut *mut TmpReentrantHdr
) {
	while !(unsafe{(*hdrp).is_null()}) {
		let nextpos: *mut TmpReentrantHdr = unsafe {
			(**hdrp).next
		};
		use std::ffi::c_void;
		use std::ptr::NonNull;
		let to_free: NonNull<c_void> = unsafe {
			NonNull::new_unchecked((*hdrp).cast::<c_void>())
		};
		let to_size: usize = unsafe {
			(*(*hdrp)).size
		};
		use nix::sys::mman::munmap;
		unsafe {
			munmap(to_free, to_size)
    				.expect("munmap failed with error");
		};
		unsafe {
			*hdrp = nextpos;
		}
	}
}

fn main() {
	let mut memctrl: *mut TmpReentrantHdr = std::ptr::null_mut();
	for incsize in 1..=35215 {
		let tmpalloc: *mut i64 = unsafe {
			tmpmap(&raw mut memctrl, incsize * size_of::<i64>())
		};
		for i in 0..incsize {
			unsafe {*(tmpalloc.add(i)) = 1;}
		}
		unsafe {tmpmapfree(&raw mut memctrl)};
	}
}

And here's the C

#include <errno.h>
#include <stddef.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/mman.h>

typedef struct PrivTmpReentrantHdr {
	size_t size;
	struct PrivTmpReentrantHdr* next;
} TmpReentrantHdr;

void* tmpmap(
	TmpReentrantHdr** markp,
	size_t rsize
) {
	size_t tsize = (rsize+sizeof(TmpReentrantHdr))+(8-rsize%8)%8;
	void* bptr = mmap(
		NULL,
	 	tsize,
		PROT_READ|PROT_WRITE,
	 	MAP_ANONYMOUS|MAP_PRIVATE,
		-1,
		0
	);
	if (bptr == MAP_FAILED) {
		fprintf(
			stderr,
			"mmap failure with error %d %s \n",
			errno,
			strerror(errno)
		);
		abort();
	}
	TmpReentrantHdr* hptr = (TmpReentrantHdr*) bptr;
	hptr->size = tsize;
	hptr->next = *markp;
	*markp = hptr;
	hptr++;
	void* rptr = (void*) hptr;
	return rptr;
}

void tmpmapfree(
	TmpReentrantHdr** hdrp
) {
	while (*hdrp != NULL) {
		TmpReentrantHdr* nextpos = (*hdrp)->next;
		size_t sizetrace = (*hdrp)->size;
		int handle = munmap((void*) *hdrp, sizetrace);
		if (handle == -1) {
			fprintf(
				stderr,
				"munmap failure with error %d %s \n",
				errno,
				strerror(errno)
			);
			abort();
		}
		else {
			*hdrp = nextpos;
		}
	}
	return;
}

int main(void) {
	TmpReentrantHdr* memctrl = NULL;
	for (size_t incsize = 1; incsize < 35215; incsize++) {
		int64_t* tmpalloc = tmpmap(&memctrl, sizeof(int64_t) * incsize);
		*tmpalloc = 2;
		for (int i=0; i < incsize; i++) {
			tmpalloc[i] = 1;
		}
		tmpmapfree(&memctrl);
	}
	return 0;
}

The C example runs like a bliss on my machine, the Rust example segfaults, systematically, at incsize = 481 (in function main), attempting to read i=480, which should be valid, just like when it sucessfully wrote to i=479 when incsize was 480, one cycle prior.

Heeeeelp

	let rptr: *mut T = unsafe {
		hptr
			.add(size_of::<TmpReentrantHdr>())
			.cast::<T>()
	};

hptr seems to be of type *mut TmpReentrantHdr and ptr::add already multiplies the passed value by the size of the pointed to type. This is actually adding size_of::<TmpReentrantHdr> * size_of::<TmpReentrantHdr> to the pointers underlying value.

480 * sizeof(u64) + sizeof(TmpReentrantHdr) * sizeof(TmpReentrantHdr) = 4096. mmap is giving you a full multiple of 4096 byte pages regardless of what you ask for. Your T pointer is starting way past the end of the header, and once you hit item 480 you are accessing past the end of the mapped page.

As mentioned already, the equivalent of hptr++ is hptr = unsafe { hptr.add(1) }, not .add(size_of::<PointeeOfHptr>()). (Maybe this comparison makes the rationale more obvious?)

Ah heeeell nah, so the problem was that I can't read and forgot about automatic pointer alignment with the add method. It always feels terrible when doing a mistake like that despite knowing about how it actually works haha

Thanks for pointing it out !! I was wondering whether there was some kind of padding I had mistakenly put... turns out it was hiding in plain sight ! One thing fixed. However, memusage still indicates a stack peak of 1072 bytes and 2500 bytes of heap allocations in the Rust version. This looks like bloat to me, but I really do not know where on Earth is malloc called in my program

Yeah, indeed this was the problem. Thanks for helping, too !