Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Why does free crash when called twice?

Tags:

In C and C++, free(my_pointer) crashes when it is called twice.

Why? There is bookkeeping of every malloc along with the size. When the first free is called then it identifies that this was allocated with what size that's why we need not to pass size along with free call.

Since it knows every thing why doesn't it check the second time around and do nothing?

Either I don't understand malloc/free behavior or free is not safely implemented.

like image 369
vikas jain Avatar asked Jun 25 '10 11:06

vikas jain


2 Answers

You are not allowed to call free on unallocated memory, the standard states that quite clearly (slightly paraphrased, my emphasis):

The free function causes the space pointed to by its argument to be deallocated, that is, made available for further allocation. If the argument is a null pointer, no action occurs. Otherwise, if the argument does not match a pointer earlier returned by a memory management function, or if the space has been deallocated by a call to free or realloc, the behavior is undefined.

What happens, for example, if the address you're double-freeing has been reallocated in the middle of a new block and the code that allocated it just happened to store something there that looked like a real malloc-block header? Like:

 +- New pointer    +- Old pointer  v                 v +------------------------------------+ |                  <Dodgy bit>       | +------------------------------------+ 

Chaos, that's what.

Memory allocation functions are a tool just like a chainsaw and, provided you use them correctly, you should have no problems. If you misuse them, however, the consequences are your own fault, either corrupting memory or worse, or cutting off one of your arms :-)


And regarding the comment:

... it can communicate gracefully to enduser about the doubling free the same location.

Short of keeping a record of all malloc and free calls to ensure you don't double-free a block, I can't see this as being workable. It would require a huge overhead and still wouldn't fix all the problems.

What would happen if:

  • thread A allocated and freed memory at address 42.
  • thread B allocated memory a address 42 and started using it.
  • thread A freed that memory a second time.
  • thread C allocated memory a address 42 and started using it.

You then have threads B and C both thinking they own that memory (these don't have to be threads of execution, I'm using the term thread here as just a piece of code that runs - it could all be in the one thread of execution but called sequentially).

No, I think the current malloc and free are just fine provided you use them properly. By all means give some thought to implementing your own version, I see nothing wrong with that but I suspect you'll run into some pretty thorny performance issues.


If you do want to implement your own wrapper around free, you can make it safer (at the cost of a little performance hit), specifically with something like the myFreeXxx calls below:

#include <stdio.h> #include <stdlib.h>  void myFreeVoid (void **p) { free (*p); *p = NULL; } void myFreeInt  (int  **p) { free (*p); *p = NULL; } void myFreeChar (char **p) { free (*p); *p = NULL; }  int main (void) {     char *x = malloc (1000);     printf ("Before: %p\n", x);     myFreeChar (&x);     printf ("After:  %p\n", x);     return 0; } 

The upshot of the code is that you can call myFreeXxx with a pointer to your pointer and it will both:

  • free the memory; and
  • set the pointer to NULL.

That latter bit means that, if you try to free the pointer again, it will do nothing (because freeing NULL is specifically covered by the standard).

It won't protect you from all situations, such as if you make a copy of the pointer elsewhere, free the original, then free the copy:

char *newptr = oldptr; myFreeChar (&oldptr);     // frees and sets to NULL. myFreeChar (&newptr);     // double-free because it wasn't set to NULL. 

If you're up to using C11, there's a better way than having to explicitly call a different function for each type now that C has compile time function overloading. You can use generic selections to call the correct function while still allowing for type safety:

#include <stdio.h> #include <stdlib.h>  void myFreeVoid (void **p) { free (*p); *p = NULL; } void myFreeInt  (int  **p) { free (*p); *p = NULL; } void myFreeChar (char **p) { free (*p); *p = NULL; } #define myFree(x) _Generic((x), \     int** :  myFreeInt,  \     char**:  myFreeChar, \     default: myFreeVoid  )(x)  int main (void) {     char *x = malloc (1000);     printf ("Before: %p\n", x);     myFree (&x);     printf ("After:  %p\n", x);     return 0; } 

With that, you simply call myFree and it will select the correct function based on the type.

like image 90
paxdiablo Avatar answered Oct 28 '22 00:10

paxdiablo


You might be misinterpreting its behavior. If it crashes right away then it is implemented in a safe manner. I can attest that this was not common behavior for free() many moons ago. The typical CRT implementation back then did no checking at all. Fast and furious, it would simply corrupt the heap's internal structure, messing up the allocation chains.

Without any diagnostic at all, the program would then misbehave or crash long after the heap corruption took place. Without having any hint why it misbehaved that way, the code that crashed wasn't actually responsible for the crash. A heisenbug, very difficult to troubleshoot.

This is not common anymore for modern CRT or OS heap implementations. This kind of undefined behavior is very exploitable by malware. And it makes your life a wholeheckofalot easier, you'll quickly find the bug in your code. It has kept me out of trouble for the past few years, haven't had to debug untraceable heap corruption in a long time. Good Thing.

like image 34
Hans Passant Avatar answered Oct 27 '22 23:10

Hans Passant