malloc results in segmentation fault after mprotect

Question

I'm getting a segmentation fault the first time I call malloc() after I protect a memory region with mprotect(). This is a code sniplet that does the memory allocation the the protection:

#define PAGESIZE 4096
void* paalloc(int size){   // Allocates and aligns memory
        int type_size =  sizeof(double);
        void* p;
        p = malloc(type_size*size+PAGESIZE-1);
        p = (void*)(((long) p + PAGESIZE-1) & ~(PAGESIZE-1));
        return p;
}
void aprotect(int size, void* array){  // Protects memory after values are set
        int type_size = sizeof(double);
        if (mprotect(array, type_size*size, PROT_READ)) {
                perror("Couldn't mprotect");
        }
}

I want to use mprotect to avoid anything writing into my arrays (which are pre-calculated sine/cosine values). Is this a stupid idea?

Answer 1

caf has the cause of the problem nailed down.

I think you can do without the mprotect(): If it is that important to you declare the pointer as static in your lookup.c (or whatever it is called) and then have a non-static double get_sine(int index); function. This way, code outside of lookup.c can only call get_sine(), and not access the table directly.

Also, From my mprotect() man page:

POSIX says that mprotect() can be used only on regions of memory obtained from mmap(2)

(Apparently this does not count for Linux. Which operating system are you using?)

Answer 2

mprotect can only work in units of pages, as you probably already know. In this case, you're correctly aligning the start of your block to a page boundary, but what you're not doing is ensuring that your allocation extends to the end of the last page you're going to use in it.

This means that your mprotect is protecting data past the end of your allocation (right to the end of that page), which is space that the next malloc call assumes it can write to.

The easiest fix is to change the PAGE_SIZE - 1 in the malloc call to PAGE_SIZE * 2.

Answer 3

I would recommend you just use mmap directly to create an anonymous mapping, and then call mprotect on that after you finishing writing to the array. Because you are always allocating entire pages, it doesn't make sense to use the heap at all. It's primary purpose is to deal with allocation and deallocation of small(ish) objects. When dealing with page blocks it just adds unnecessary overhead.