-3

I have a test for this situation:

 // Diabolical wild free #2.

int main() {
 char* a = (char*) my_malloc(200);
 char* b = (char*) my_malloc(50);
 char* c = (char*) my_malloc(200);
 char* p = (char*) my_malloc(3000);
 (void) a, (void) c;
 memcpy(p, b - 200, 450);
 my_free(b);
 memcpy(b - 200, p, 450);
 my_free(b);
 m61_printstatistics();
}

Expected result:

//! MEMORY BUG: invalid free of pointer

I now that we can use free list to check second free() call, but

1) second free() call, free another object, because memcpy(b - 200, p, 450)

2) In this situation we also call multiple times free() on the same object, but this is correct execution

// A correct execution should not report errors.

int main() {
 for (int i = 0; i < 10; ++i)
 {
     int* ptr = (int*) my_malloc(sizeof(int) * 10);
     for (int j = 0; j < 10; ++j)
     {
         ptr[i] = i;
     }
     my_free(ptr);
 }
 m61_printstatistics();
 }

So how to check invalid free of pointer in free() implementation?

Anatoly
  • 1,824
  • 2
  • 17
  • 42
  • 3
    After calling `free()` you can set the pointer to `NULL` which will make the second call to `free()` a non op. Calling `free()` on something that has already been freed is undefined behavior. – NathanOliver Apr 30 '15 at 17:25
  • 1
    `free` does stuff beyond what you have (valid) access to. You cannot free the same block twice. http://linux.die.net/man/3/free: "..which must have been returned by a previous call to malloc(), calloc() or realloc(). Otherwise, or if free(ptr) has already been called before, undefined behavior occurs." – Jongware Apr 30 '15 at 17:29
  • 1
    @NathanOliver This is not work. Beacuse we `memcpy(b - 200, p, 450)` and all the data will be destory – Anatoly Apr 30 '15 at 17:29
  • @NathanOliver - He wants to signal exactly this "second call to free()", if i get it right :) – SChepurin Apr 30 '15 at 17:29
  • 1
    BTW, that 2nd memcpy is completely undefined behavior as well. You can't touch memory after it is freed. – Michael Dorgan Apr 30 '15 at 17:30
  • 1
    @MichaelDorgan I touch another block of memory! because `memcpy(b - 200, p, 450)` – Anatoly Apr 30 '15 at 17:33
  • 3
    @Anatoly You seem to think that memcpy physically moves blocks of physical memory (or the virtual memory equivalent). It doesn't. It have as a requirement that you have the rights to both the source and destination memory block. You are entirely responsibly for insuring that you meet this requirement, but you are blatantly violating that in your code. This is a serious bug. – Avi Berger Apr 30 '15 at 18:31
  • You may be interested in this: http://stackoverflow.com/q/1656227/315052 – jxh Apr 30 '15 at 18:32
  • @AviBerger Can you explain please, because I think that memcpy copy block of memory – Anatoly Apr 30 '15 at 18:38
  • 1
    What happens if, after your free(b), but before the memcpy() occurs, if another thread/process decides to allocate that memory for its use? Then, your memcpy has now destroyed something else's memory and made a very hard to track bug. This is one reason why playing with memory after it is freed is illegal and undefined. – Michael Dorgan Apr 30 '15 at 18:45
  • @MichaelDorgan Can you see my second example? I'm playing with memory after it is freed and its ok – Anatoly Apr 30 '15 at 18:48
  • @Anatoly You provide memcpy() with 2 adequately-sized, non-overlapping blocks of memory. memcpy() copies the contents of the source block to the destination block. Descriptions of it might omit explicitly using the word "contents" since it is obvious to many of us that that is what it has to be. – Avi Berger Apr 30 '15 at 18:54
  • 1
    Looking over your post again, perhaps you are imagining that the buffers you are allocating with malloc() will be allocated sequentially and contiguously in memory. That is a false expectation. b-200 is a more or less random location. You have no clue what, if anything is there. There might not even be any memory mapped there. It didn't initially occur to me that you might not know that. – Avi Berger Apr 30 '15 at 19:31
  • @AviBerger I'm writed my own malloc implementation and it allocated sequentially and contiguously in memory. – Anatoly Apr 30 '15 at 21:43
  • Please spend more effort to explain the problem in point 1. I fail to understand point 2, since a `malloc` will obviously remove something from the free list. That is how an allocator works. It "populates" a free list, and removes items from that list to satisfy `malloc` requests. – jxh May 01 '15 at 23:05

3 Answers3

3

Let's assume we are overlooking the obvious undefined behavior caused by reading and writing beyond the object boundary pointed to by b, and that your intended purpose is to write or test a C library that is supposed to be able to diagnose the error of the double call to free() on b.

The reason this is an error is that typical implementations of malloc and free will place free()'d memory into a free memory list as a linked list. As a way to conserve memory, the just free()'d memory itself is reused as the node object for the free list data structure. Thus, some other node in the free list may be pointing to the memory represented by b after the first free() call. Upon the second free() call, the node for b will be inserted into the list again, perhaps causing a different node to be pointing to it. The free list would be considered corrupted. Since malloc() removes data from the free list, having the same node twice in the free list would be a serious problem.

There are a few ways to "catch" the problem. One possibility is that the free list be implemented as a lookup tree that uses its own memory for nodes that point to the memory returned to free(). This allows the second call to free() to notice that b is already in the tree. Thus, the double call to free() is detected and can be flagged accordingly.

jxh
  • 64,506
  • 7
  • 96
  • 165
  • 1
    When I do second free I touch another block of memory! because memcpy(b - 200, p, 450) – Anatoly Apr 30 '15 at 17:37
  • 1
    That is why the free list needs to be implemented as a separate data structure that points to the freed elements. Your test is designed to destroy the "node" data that would typically be used to store freed elements in the free list. – jxh Apr 30 '15 at 17:39
1

You can use a xfree macro for testing (assuming your program does not already free(NULL):

#if MY_DEBUG
#define xfree(p) ((void) ((p) ? (free(p), (p) = 0) : (fprintf(stderr, "double free\n"), exit(1), (void *) 0)))
#else
#define xfree(p)  free(p)
#endif
ouah
  • 134,166
  • 14
  • 247
  • 314
1

In practice, the best way to check for this sort of thing is to run it using valgrind. You can try and create your own framework/macros, but it's normally better to just include running it under valgrind as part of your periodic testing process.

Gwyn Evans
  • 1,231
  • 6
  • 17