Why is fread setting the output pointer to NULL and causing a stack-smashing error?

I asked this earlier but failed to provide a minimally reproducible example. I appreciate the feedback. I am trying to write to a binary file an int followed by an array of bools, where the int represents the length of that array.

The following code compiles and seems to produce the binary file correctly. When fread is called it sets the void* argument I pass it to NULL and triggers a stack-smashing error.

example.c

#include <assert.h>
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>

typedef struct cutlogTag{
    int len;
    bool *parts_cut;
} Cutlog;


size_t save_cutlog(const char *path, const Cutlog *p){
    
    FILE *fp;
    size_t written = 0;

    fp = fopen(path, "wb");
    if (fp == NULL){
        fprintf(stderr, "Failed to save cutlog file\n");
        return 0;
    }
    written = fwrite(&(p->len), sizeof(p->len), 1, fp);
    written += fwrite(p->parts_cut, sizeof(bool), p->len, fp);
    if(written != 1 + p->len)
        fprintf(stderr, "error writing file\n");
    else fprintf(stdout, "cutlog written to %s\n", path);
    fclose(fp);
    return written;
}

//returns cutlog with length of -1 on failure to load log
Cutlog load_cutlog(const char *path){
    
    Cutlog ret;
    FILE *fp;
    size_t read = 0;

    ret.len = -1;
    ret.parts_cut = NULL;
    
    fp = fopen(path, "rb");
    assert(fp != NULL);

    fseek(fp, 0, SEEK_SET);
    fread(&ret.len, sizeof(ret.len), 1, fp);
    ret.parts_cut = malloc(sizeof(bool) * ret.len);
    assert(ret.parts_cut);
    read = fread(&ret.parts_cut, sizeof(bool), ret.len, fp);
    if(read != ret.len){
        fprintf(stderr, "read unexpected size of data\n");
        ret.len = -1;
    }
    if (getc(fp) != EOF){
        fprintf(stderr, "expected file end. something went wrong. \n");
        ret.len = -1;
    }
    fclose(fp);
    return ret;
}

int main(int argc, char *argv[]){
    Cutlog clog;
    const char* path = "testbinary";
//initialize cutlog struct
    clog.len = 687;
    clog.parts_cut = malloc(sizeof(bool) * clog.len );
    assert(clog.parts_cut);
    for (int i = 0; i < clog.len; i++){
        clog.parts_cut[i] = false;
    }
//save to binary file and free from memory
    save_cutlog(path, &clog);
    free(clog.parts_cut);
//load from binary file
    clog = load_cutlog(path);
    fprintf(stdout, "len is %d\n", clog.len);
    return 0;
}

Tried to write a binary file an int followed by an array of bools, where the int represents the length of that array, then load the file back.

The file is written correctly but on reading it I cause a stack smashing crash.

>Solution :

read = fread(&ret.parts_cut, sizeof(bool), ret.len, fp);

&ret.parts_cut is the address of ret.parts_cut, on the stack. That is a single pointer. So, if you have more booleans that will fit in the space of a pointer, you will damage other things on the stack.

But even if it does fit, you will almost certainly still get incorrect behaviour because your pointer is now set to something not pointing to the allocated memory, some weird alias of a set of booleans, like 0x0101000101000001.

This is unlikely to end well if you later try to dereference it with something like *(ret->parts_cut).

You should be using ret.parts_cut, which is the value of the pointer, pointing to the thing you just allocated:

read = fread(ret.parts_cut, sizeof(bool), ret.len, fp);

And just a quick note: it may not be the best idea to rely on the assert macro to catch things that may cause problems. If NDEBUG is defined (which it may well be in production code), the assert macro does exactly nothing.

You’re better off using if (! condition) handle_it(), rather than assert(condition), to handle these situations.