diffent image file size causes segmentation fault when memory is freed

Question

I was learning how to add textures in OpenGL , and decided for educational purposes to not use a library like "stb_image.h" to load images , but instead to write my own one for .bmp images.

The dimensions of the image I was using is 1920x1080 (full HD) and the file size was 6.2 MB (6,220,922 bytes). After loading the image and converting it from blue,green,red (BGR) to red,green,blue (RGB) I decided to free the BGR version but when doing that a "segmentation fault (core dumped)" happened.

One of the stuff I tried was to use a different image and this causes the error to go away. The new image resolution is 1280x1920, file size of 7.4 MB (7,372,922 bytes).

I decided to flip the dimensions of the earlier image (from 1920x1080 to 1080x1920) and the "segmentation fault (core dumped)" went back.

This is the relevant code (removed all OpenGL and GLFW stuff) in C.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <stdint.h>

typedef struct 
{
   uint16_t type;
   uint32_t size;
   uint16_t reserved1;
   uint16_t reserved2;
   uint32_t offbit;
}BITMAPFILEHEADER;

typedef struct {
    uint32_t size;
    uint32_t width;
    uint32_t height;
    uint16_t planes;
    uint16_t bitcount;
    uint32_t comprestiontype;
    uint32_t sizeimage;
    uint32_t xres;
    uint32_t yres;
    uint32_t crlused;
    uint32_t crlimportant;
}BITMAPINFOHEADER;

typedef struct{
    uint8_t r;
    uint8_t g;
    uint8_t b;
}RGB;

void ReadFileHeader(FILE* file, BITMAPFILEHEADER* fileHeader){
    fread(&(fileHeader->type),2,1,file);
    fread(&(fileHeader->size),4,1,file);
    fread(&(fileHeader->reserved1),2,1,file);
    fread(&(fileHeader->reserved2),2,1,file);
    fread(&(fileHeader->offbit),4,1,file);
}

void ReadInfoHeader(FILE* file, BITMAPINFOHEADER* fileHeader){
    fread(&(fileHeader->size),4,1,file);
    fread(&(fileHeader->width),4,1,file);
    fread(&(fileHeader->height),4,1,file);
    fread(&(fileHeader->planes),2,1,file);
    fread(&(fileHeader->bitcount),2,1,file);
    fread(&(fileHeader->comprestiontype),4,1,file);
    fread(&(fileHeader->sizeimage),4,1,file);
    fread(&(fileHeader->xres),4,1,file);
    fread(&(fileHeader->yres),4,1,file);
    fread(&(fileHeader->crlused),4,1,file);
    fread(&(fileHeader->crlimportant),4,1,file);
}


unsigned char* ReadBMP(const char * p_path, int* width, int* hight){
    FILE* image = fopen(p_path,"rb");
    if(image == NULL){
        printf("file not found\n");
    }
    BITMAPFILEHEADER file_header;
    BITMAPINFOHEADER info_header;

    ReadFileHeader(image,&file_header);
    ReadInfoHeader(image,&info_header);

    if (width != NULL)
    {
        *width = info_header.width;
    }
    
    if (hight != NULL)
    {
        *hight = info_header.height;
    }

    unsigned char* pixelData = (unsigned char*)malloc(sizeof(RGB)*info_header.width*info_header.height);
    fread(pixelData,sizeof(RGB),info_header.width*info_header.height,image);
    fclose(image);
    return pixelData;
}

unsigned char* BmpToRgb(unsigned char* input, int width,int hight){
    
    RGB* data = (RGB*)malloc(width*hight*3);
    int y = 0;
    int x = 0;
    memcpy(data,input,width*hight*3);
    for (y = 0; y < hight; y++)
    {
        for (x = 0; x < width; x++)
        {

        // the problem is probably in these 3 lines, if i comment
        // them out no "Segmentation fault (core dumped)" happens.
        data[x+(hight-y)*width].r = input[x+y*width+2];
            
        data[x+(hight-y)*width].g = input[x+y*width];
            
        data[x+(hight-y)*width].b = input[x+y*width+1];

        }
    }
    
    return (unsigned char*)data; 
}

int main(){

    int width;
    int hight;

    unsigned char* imageBMP = ReadBMP("image.bmp",&width,&hight);
    printf("width: %d hight: %d\n",width,hight);
    unsigned char* image = BmpToRgb(imageBMP,width,hight);

    free(image);
    printf("freeing\n");
    free(imageBMP); // segmentation fault (core dumped) for 1920x1080 images
    printf("done\n"); // "done" not printed

    return 0;
}

I want to know why different image resolutions (or maybe it is the file size) may cause a "segmentation fault (core dumped)" or not.

Some extra information that may be relevant:

Operating system : Ubuntu 20.04  
c compiler: gcc

First of all you need to use a debugger to catch crashes and locate when and where in your code it happens. I also recommend tools like Valgrind to help you find such issues. — Some programmer dude, Jan 29 '21 at 04:42
Also the size of the `RGB` structure [might not be what you expect](https://stackoverflow.com/questions/119123/why-isnt-sizeof-for-a-struct-equal-to-the-sum-of-sizeof-of-each-member), in which case you don't allocate enough memory. — Some programmer dude, Jan 29 '21 at 04:47
the size of struct RGB is 3 bytes , there is no struct padding, I checked. — Any name works, Jan 29 '21 at 04:55
`data[x+(hight-y)*width].r = input[x+y*width+2];` When `y == 0` this writes past the end of the allocated `data` buffer. The `input` indexing doesn't look right, either. — dxiv, Jan 29 '21 at 05:46
As a side note, the "width" and "hight" local variables should be defined as uint32_t (unsigned int) instead of int. You may face integer overflow otherwise. — Rachid K., Jan 29 '21 at 05:59
@RachidK. I originally made it a uint32_t , but OpenGL didn't like that , so I made it a normal int. — Any name works, Jan 29 '21 at 06:54
Welcome to Stack Overflow! Please do not edit solution announcements into the question. Accept (i.e. click the "tick" next to it) one of the existing answer, if there are any. You can also create your own answer, and even accept it, if your solution is not yet covered by an existing answer. — Yunnosch, Jan 29 '21 at 07:18
If your segfault problem is solved and you want to refocus your question you can do that as long as there are no anwers which are invalidated by the change. But please do not keep it as an question about segfault which is then answered within the question and then changed. — Yunnosch, Jan 29 '21 at 07:20
@dxiv thanks for stating that. I changed it `data[x+(hight-y-1)*width].r = input[x+y*width+2];` which solved the "segmentation fault (core dumped)" — Any name works, Jan 29 '21 at 07:32
now all that remains is the other question "I want to know why different image resolutions (or maybe it is the file size) may cause a "segmentation fault (core dumped)" or not." — Any name works, Jan 29 '21 at 07:35
@Anynameworks Once you overrun the buffer, even by just one byte, you are in [undefined behavior](https://en.cppreference.com/w/c/language/behavior) territory, meaning that anything is allowed to happen, so there isn't much point wondering why any particular outcome happened. — dxiv, Jan 29 '21 at 08:05

score 1 · Accepted Answer · answered Jan 29 '21 at 08:23

the problem was with these lines

data[x+(hight-y)*width].r = input[x+y*width+2];
            
data[x+(hight-y)*width].g = input[x+y*width];

data[x+(hight-y)*width].b = input[x+y*width+1];

when y = 0 the buffer for data is overrun. This makes the code enter in an undefined behavior state (https://en.cppreference.com/w/c/language/behavior.)

to solve this problem replace the three lines shown above with the following

data[x+(hight-y-1)*width].r = input[x+y*width+2];
            
data[x+(hight-y-1)*width].g = input[x+y*width];

data[x+(hight-y-1)*width].b = input[x+y*width+1];

diffent image file size causes segmentation fault when memory is freed

1 Answers1