Search code examples
cmemory-managementvalgrindfree

"Invalid write of size 1" and pointer has right memory address but still free throws error


I have below code which gives me 2 errors - Invalid write of size 1 and Address 0x41f52a8 is 0 bytes after a block of size 128 alloc'd. Below is the complete valgrind stack.

I could do a guess work and identify that memcpy(content2 + totalLength + 1, fileContentTemp, readBytes); line of code has problem. So, I corrected it to memcpy(content2 + totalLength, fileContentTemp, readBytes); and then my valgrind became really happy and all pass.

But I am not able to understand the reason. To me it looks that from 2nd time onwards I need to do + 1 because I would not like memcpy to start writing from content2 + totalLength address since it is the address where last byte was written so I thought to increment and starting from next address.

Also, another intriguing part is that if I do not correct it then I get following runtime error *** Error in ./server_issue': double free or corruption (!prev): 0x08c24170 ***, I guess coming because of this line of code free(content2);. free basically free the pointer, now if pointer is having correct memory address (I verified that content2 has right memory address) then why exception occurs.

Code:

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

typedef char BYTE;
bool load(FILE*, BYTE**, size_t*);

int main(void) {
    FILE *file = fopen("/home/jharvard/psets/pset6/pset6_working/public/hello.html", "r");
    BYTE *content;
    size_t length;
    load(file, &content, &length);
}

bool load(FILE *file, BYTE **content, size_t *length) {
    //printf("file %p\n", file);

    int totalLength = 0;
    int readBytes = 0;
    BYTE *content2 = NULL;

    BYTE *fileContentTemp[64]; // working with 222222
    while ((readBytes = fread(fileContentTemp, 1, 64, file)) > 0) {
        printf("Reallocating %d bytes, ", readBytes);
        content2 = realloc(content2, sizeof(BYTE) * (totalLength + readBytes));

        printf("%p\n", content2);
        if (totalLength != 0) {
            memcpy(content2 + totalLength + 1, fileContentTemp, readBytes);        
        } else {
            memcpy(content2 + totalLength, fileContentTemp, readBytes);        
        }
        totalLength = totalLength + readBytes;
    }

    *length = totalLength;
    *content = content2;

    free(content2);

    //printf("CC image: %s\n", *content);
    //printf("length is %d\n", *length);
    //printf("fileContent %p\n", *content);
    //printf("file %p\n", file);

    fclose(file);

    //printf("length is %d\n", *length);

    return true;
}

Valgrind stack:

appliance (~/psets/pset6/pset6_working): valgrind ./server_issue
==3206== Memcheck, a memory error detector
==3206== Copyright (C) 2002-2013, and GNU GPL'd, by Julian Seward et al.
==3206== Using Valgrind-3.10.1 and LibVEX; rerun with -h for copyright info
==3206== Command: ./server_issue
==3206== 
Reallocating 64 bytes, 0x41f51b8
Reallocating 64 bytes, 0x41f5228
==3206== Invalid write of size 1
==3206==    at 0x402F04B: memcpy (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==3206==    by 0x80486AC: load (in /home/jharvard/psets/pset6/pset6_working/server_issue)
==3206==    by 0x8048599: main (in /home/jharvard/psets/pset6/pset6_working/server_issue)
==3206==  Address 0x41f52a8 is 0 bytes after a block of size 128 alloc'd
==3206==    at 0x402C324: realloc (in /usr/lib/valgrind/vgpreload_memcheck-x86-linux.so)
==3206==    by 0x804865C: load (in /home/jharvard/psets/pset6/pset6_working/server_issue)
==3206==    by 0x8048599: main (in /home/jharvard/psets/pset6/pset6_working/server_issue)
==3206== 
Reallocating 64 bytes, 0x41f52d8
Reallocating 64 bytes, 0x41f53c8
Reallocating 60 bytes, 0x41f54f8
==3206== 
==3206== HEAP SUMMARY:
==3206==     in use at exit: 0 bytes in 0 blocks
==3206==   total heap usage: 6 allocs, 6 frees, 1,308 bytes allocated
==3206== 
==3206== All heap blocks were freed -- no leaks are possible
==3206== 
==3206== For counts of detected and suppressed errors, rerun with: -v
==3206== ERROR SUMMARY: 4 errors from 1 contexts (suppressed: 0 from 0)

Solution

  • This comes down to the fact that a list of the first N integers, starting from 0, ends at N-1.

    Let's try a concrete example. Suppose you write 4 bytes from index 0. You write indices 0, 1, 2 and 3. That's 4 bytes in total. So the next one to right is index 4, and not 5.

    Suppose you write N bytes from index M. You write M, M+1, ..., M+N-1. The next one you write is M+N.

    So replace the entire if statement with

    memcpy(content2 + totalLength, fileContentTemp, readBytes); 
    

    The if should have felt wrong. An operation like that should not need special casing, so the fact you had an if should have raised alarm bells.