CODEWITHSUNDEEP
cmemory-managementgarbage-collectionstackfree
vidicon
vidicon

Reputation: 123

free() on stack memory

I'm supporting some c code on Solaris, and I've seen something weird at least I think it is:

char new_login[64];
...
strcpy(new_login, (char *)login);
...
free(new_login);

My understanding is that since the variable is a local array the memory comes from the stack and does not need to be freed, and moreover since no malloc/calloc/realloc was used the behaviour is undefined.

This is a real-time system so I think it is a waste of cycles. Am I missing something obvious?

Upvotes: 12

Views: 19578

Answers (5)

nos
nos

Reputation: 229204

You can only free() something you got from malloc(),calloc() or realloc() function. freeing something on the stack yields undefined behaviour, you're lucky this doesn't cause your program to crash, or worse.

Consider that a serious bug, and delete that line asap.

Upvotes: 26

Dan B
Dan B

Reputation: 281

The free() is definitely a bug.
However, it's possible there's another bug here: 


   strcpy(new_login, (char *)login);

If the function isn't pedantically confirming that login is 63 or fewer characters with the appropriate null termination, then this code has a classic buffer overflow bug. If a malicious party can fill login with the right bytes, they can overwrite the return pointer on the stack and execute arbitrary code. One solution is:


   new_login[sizeof(new_login)-1]='\0';
   strncpy(new_login, (char *)login, sizeof(new_login)-1 );

Upvotes: 4

Yktula
Yktula

Reputation: 14819

IN MOST CASES, you can only free() something allocated on the heap. See http://www.opengroup.org/onlinepubs/009695399/functions/free.html .

HOWEVER: One way to go about doing what you'd like to be doing is to scope temporary variables allocated on the stack. like so:

{
char new_login[64];
... /* No later-used variables should be allocated on the stack here */
strcpy(new_login, (char *)login);
}
...

Upvotes: 4

jweyrich
jweyrich

Reputation: 32260

Definitely a bug. free() MUST ONLY be used for heap alloc'd memory, unless it's redefined to do something completely different, which I doubt to be the case.

Upvotes: 3

Paul Nathan
Paul Nathan

Reputation: 40319

No. This is a bug.

According to free(3)....

free() frees the memory space pointed to by ptr, 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 behaviour occurs. If ptr is NULL, no operation is performed.

So you have undefined behavior happening in your program.

Upvotes: 9

Related Questions