Error when freeing memory in C

Question

I am parsing a lot of data and I am using C to it. It works for almost all the data but at one point I get the error:

  *** glibc detected *** ./a.out: free(): invalid next size (fast): 0x091fb288 ***
  ======= Backtrace: ========= 
  /lib/i386-linux-gnu/libc.so.6(+0x75ee2)[0xb75d1ee2]
  ./a.out[0x8049321]
  ./a.out[0x80494b3]
  ./a.out[0x804b843]
  /lib/i386-linux-gnu/libc.so.6(__libc_start_main+0xf3)[0xb75754d3]

My code is the following. The conditional is in a loop that iterates successfully for many iterations, but at one point throws the error. My error occurs when free(tmp); is called, however, tmp is only used in this small area of the code.

...
if(tokens_o[i].start != tokens_o[i].end)
            {
                    tmp = printToken(content, &tokens_o[i]);
                    printf("%s \n", tmp);
                    free(tmp);
            }
 ...

char *
printToken(char *text, jsmntok_t *token)
{
    int size = token->end - token->start;
    char *text_token = calloc(size+1, sizeof(char));
    if(text_token == NULL)
    {
        printf("error when reading token \n");
        exit(0);
    }
    strncpy(text_token, text+token->start, size);

    return text_token;
}

The actual source of the error may be elsewhere, but it's not until `free` is called before the error is actually detected. — Drew McGowen, Jul 30 '14 at 20:46
Does that imply that you don't see any troubles with snippet of code? — user1090614, Jul 30 '14 at 20:51
This probably won't fix the error, but stylistically, you forgot the cast of `(char*)` on `calloc`. Just makes your intentions clear. — bhuvy, Jul 30 '14 at 20:51
@bhuvan-venkatesh actually, casting the return value of `malloc`/`calloc` is bad practice — Drew McGowen, Jul 30 '14 at 20:52
@Drew-McGowen Sorry for my outdated and wrong practice (non sarcastically), thank you. — bhuvy, Jul 30 '14 at 20:54
@DavidFrye: See here: [Don't cast the result of malloc (and friends)](http://stackoverflow.com/questions/605845). — Deduplicator, Jul 30 '14 at 20:56
Can you run it under valgrind and post the results? Even GDB might give you some more useful info, if you recompile with `-ggdb`. — Patrick Collins, Jul 30 '14 at 21:06
Thanks to Drew McGowen and Patrick Collins - the error was elsewhere and I found it using Valgrind (pretty cool tool!!!). — user1090614, Jul 31 '14 at 02:50

score 3 · Answer 1 · answered Jul 30 '14 at 21:11

3

I expect that in one of the iterations the calculated size is -1. This means that calloc would be called with with nmemb set to 0.

According to the man page, calloc can return a unique pointer value that can later be successfully passed to free if it is called with nmemb as 0.

strncpy is then called with size -1 but the type of n is size_t (i.e. unsigned) and strncpy will therefore write outside any allocated boundaries.

answered Jul 30 '14 at 21:11

Terry Santegoeds

582
3
6

If `calloc` returns a unique pointer value that can be successfully passed to `free`, why is it crashing when he passes it to `free`? Whether or not `strncpy` tries to write something to it shouldn't make a difference as far as `free` is concerned. – Patrick Collins Jul 30 '14 at 21:13
malloc/calloc/free maintain information wrt the blocks of memory that have been allocated. If this information gets corrupted it is very possible that a call to free crashes the program. – Terry Santegoeds Jul 30 '14 at 21:18
Thank you for the input Tjerk, but this is sadly not the case (size is always > 0, I just checkd)... – user1090614 Jul 30 '14 at 21:19
@Deduplicator I'm not sure there is UB here. `strncpy` writes at most `n` characters. If it was given 0 or -1, it should write 0 characters and so not touch the pointer. [This snippet](http://stackoverflow.com/questions/14065391/strncpy-leading-to-segmentation-fault) of the standard doesn't mention UB in the case of a negative size. Some corruption of malloc/free's bookkeeping data makes a lot of sense though -- maybe somewhere that we're not seeing here. Valgrind would catch it, if it's there. – Patrick Collins Jul 30 '14 at 21:20
@Patrick: All library functions, unless explicitly specified, expect non-null pointers. Also, how should it know you meant -1 and not some humungeously big positive number? – Deduplicator Jul 30 '14 at 21:42
@Deduplicator ah, because it takes a `size_t` and `size_t` is unsigned. You're right then. – Patrick Collins Jul 30 '14 at 21:44
There is one issue that doesn't make sense, since `-1` for `size_t` would be a very large copy, so I would have expected a faulted memory write before faulting on `free()`. – jxh Jul 30 '14 at 22:12
There could be heap corruption in code that is not shown, that just shows up here – M.M Jul 30 '14 at 22:23

Error when freeing memory in C

1 Answers1