Segmentation fault with Posix-C program using mmap and mapfile

Question

Well I have this program and I get a segmentation fault: 11 (core dumped). After lots of checks I get this when the for loop gets to i=1024 and it tries to mapfile[i]=0. The program is about making a server and a client program that communicates by read/writing in a common file made in the server program. This is the server program and it prints the value inside before and after the change. I would like to see what's going on, if it's a problem with the mapping or just problem with memory of the *mapfile. Thanks!

#include <sys/shm.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <sys/mman.h>
#include <errno.h>
#include <math.h>

int main()
{
    int ret, i;           
    int *mapfile;           

    system("dd if=/dev/zero of=/tmp/c4 bs=4 count=500");

    ret = open("/tmp/c4", O_RDWR | (mode_t)0600);
    if (ret == -1)
    {
        perror("File");
        return 0;
    }

    mapfile = mmap(NULL, 2000, PROT_READ | PROT_WRITE, MAP_SHARED, ret, 0);

    for (i=1; i<=2000; i++)
    {
        mapfile[i] = 0;
    }

    while(mapfile[0] != 555)
    {
        mapfile = mmap(NULL, 2000, PROT_READ | PROT_WRITE, MAP_SHARED, ret, 0);
        if (mapfile[0] != 0)
        {
            printf("Readed from file /tmp/c4 (before): %d\n", mapfile[0]);
            mapfile[0]=mapfile[0]+5;
            printf("Readed from file /tmp/c4 (after)  : %d\n", mapfile[0]);
            mapfile[0] = 0;
        }
        sleep(1);
    }

    ret = munmap(mapfile, 2000);
    if (ret == -1)
    {
        perror("munmap");
        return 0;
    }

    close(ret);

    return 0;
}

Answer 1

You're requesting 2000 bytes from mmap, but treating the returned value as an array of 2000 ints. That can't work, an int is usually 4 or 8 bytes these days. You'll be writing past the end of the reserved memory in your loop.

Change the mmap calls to use 2000*sizeof(int). And while you're at it, give that 2000 constant a name (e.g. const int num_elems = 2000; near the top) and don't repeat the magic constant all over the place. And once that's done change it to 1024 or 2048 so that the resulting size is a multiple of the page size (if you're not sure of your page size, getconf PAGE_SIZE on the command line).

And also change your dd command to create a large-enough file. It is currently creating a 2000 byte file, you'll need to increase that as well.

And validate the return value of mmap - it can fail, and you should detect that.

Finally, don't continuously remap, you're using MAP_SHARED modifications through other shared mappings of the same file and offset will be visible to your process. (Must really be the same file, if the other process also does a dd, that might not work. Only one process should have the responsibility of creating that file.)

If you do want to remap, you must also unmap each time. Otherwise you're leaking mappings.