lastland
lastland

Reputation: 910

A "free(): invalid next size (fast)" in C++

I just ran into a free(): invalid next size (fast) problem while writing a C++ program. And I failed to figure out why this could happen unfortunately. The code is given below.

bool not_corrupt(struct packet *pkt, int size)
{
    if (!size) return false;
    bool result = true;
    char *exp_checksum = (char*)malloc(size * sizeof(char));
    char *rec_checksum = (char*)malloc(size * sizeof(char));
    char *rec_data = (char*)malloc(size * sizeof(char));
    //memcpy(rec_checksum, pkt->data+HEADER_SIZE+SEQ_SIZE+DATA_SIZE, size);
    //memcpy(rec_data, pkt->data+HEADER_SIZE+SEQ_SIZE, size);
    for (int i = 0; i < size; i++) {
        rec_checksum[i] = pkt->data[HEADER_SIZE+SEQ_SIZE+DATA_SIZE+i];
        rec_data[i] = pkt->data[HEADER_SIZE+SEQ_SIZE+i];
    }
    do_checksum(exp_checksum, rec_data, DATA_SIZE);
    for (int i = 0; i < size; i++) {
        if (exp_checksum[i] != rec_checksum[i]) {
            result = false;
            break;
        }
    }
    free(exp_checksum);
    free(rec_checksum);
    free(rec_data);
    return result;
}

The macros used are:

#define RDT_PKTSIZE 128
#define SEQ_SIZE 4
#define HEADER_SIZE 1
#define DATA_SIZE ((RDT_PKTSIZE - HEADER_SIZE - SEQ_SIZE) / 2)

The struct used is:

struct packet {
    char data[RDT_PKTSIZE];
};

This piece of code doesn't go wrong every time. It would crash with the free(): invalid next size (fast) sometimes in the free(exp_checksum); part.

What's even worse is that sometimes what's in rec_checksum stuff is just not equal to what's in pkt->data[HEADER_SIZE+SEQ_SIZE+DATA_SIZE] stuff, which should be the same according to the watch expressions from my debugging tools. Both memcpy and for methods are used but this problem remains.

I don't quite understand why this would happen. I would be very thankful if anyone could explain this to me.

Edit:

Here's the do_checksum() method, which is very simple:

void do_checksum(char* checksum, char* data, int size)
{
    for (int i = 0; i < size; i++)
    {
        checksum[i] = ~data[i];
    }
}

Edit 2:

Thanks for all.

I switched other part of my code from the usage of STL queue to STL vector, the results turn to be cool then.

But still I didn't figure out why. I am sure that I would never pop an empty queue.

Upvotes: 0

Views: 6046

Answers (5)

esskar
esskar

Reputation: 10970

DATA_SIZE is a macro defined the max length in my program so the size should be less than DATA_SIZE

even if that is true, your logic only creates enough memory to hold size characters. so you should call

do_checksum(exp_checksum, rec_data, size);

and, if you do not want to use std::string (which is fine), you should switch from malloc/free to new/delete when talking C++

Upvotes: 0

paulsm4
paulsm4

Reputation: 121881

Valgrind is good ... but validating all your inputs and checking all error conditions is even better.

Stepping through the code in the debugger isn't a bad idea, either.

I would also call "do_checksum (size)" (your actual size), instead of DATA_SIZE (presumably "maximum size").

Upvotes: 0

David Heffernan
David Heffernan

Reputation: 613572

The error you report is indicative of heap corruption. These can be hard to track down and tools like valgrind can be extremely helpful. Heap corruptions are often hard to debug with a simple debugger because the runtime error often occurs long after the actual corruption.

That said, the most obvious potential cause of your heap corruption, given the code posted so far, is if DATA_SIZE is greater than size. If that occurs then do_checksum will write beyond the end of exp_checksum.

Upvotes: 4

paulsm4
paulsm4

Reputation: 121881

Three immediate suggestions:

  1. Check for size <= 0 (instead of "!size")

  2. Check for size >= DATA_SIZE

  3. Check for malloc returning NULL

Upvotes: 1

pmg
pmg

Reputation: 108986

Have you tried Valgrind?

Also, make sure to never send more than RDT_PKTSIZE as size to not_corrupt()

bool not_corrupt(struct packet *pkt, int size)
{
    if (!size) return false;
    if (size > RDT_PKTSIZE) return false;
    /* ... */

Upvotes: 0

Related Questions