PT31 / honeyd

Automatically exported from code.google.com/p/honeyd
0 stars 0 forks source link

Honeyd memory leak when packet size is equal to HONEYD_MTU #13

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Start Honeyd with a virtual interface.
2. Ping the virtual interface with a packet size >= HONEYD_MTU (1500). I
was able to do this with the command 'ping -s 1472 <honeyd_ip>'
3. Watch the memory usage of the Honeyd process grow without bound.

What is the expected output? What do you see instead?
Memory usage should remain stable.

What version of the product are you using? On what operating system?
Honeyd 1.5c.

Please provide any additional information below.

The 'pool_pkt' structure is initialized with a pool size of HONEYD_MTU in
honeyd.c. Then in two methods in honeyd.c (honeyd_delay_packet and
icmp_echo_reply), there is the following construct:

    if (iplen < HONEYD_MTU)
        pkt = pool_alloc(pool_pkt);
    else
        pkt = pool_alloc_size(pool_pkt, iplen);

It appears that pool_alloc gives a chunk of memory equal to the pool size,
while pool_alloc_size can give a larger chunk of memory. However, in this
case if iplen == HONEYD_MTU, pool_alloc_size allocates a new chunk of
memory equal to the pool size, and then when pool_free is called on the
memory, it does:

    if (entry->size == pool->size)
        SLIST_INSERT_HEAD(&pool->entries, entry, next);
    else {
        free(entry);
        pool->nalloc--;
    }

In this case the entry is added back onto the pool->entries list, however
the next packet that comes in with the same size will cause a new
allocation instead of looking in pool->entries.

This issue can be resolved by changing the line:

    if (iplen < HONEYD_MTU)

to:

    if (iplen <= HONEYD_MTU)

However, it might be a cleaner API to remove pool_alloc_size, and just make
pool_alloc take a size parameter. Then pool_alloc will allocate from the
pool if size <= pool->size, or allocate a new chunk of memory if size >
pool->size.

After making this change, the SCP performance issue reported in issue #12
is much less pronounced. Transferring a 5 MB file to Honeyd now completes
in 11 seconds instead of 32 on my test system (most likely because it
doesn't need to allocate memory for each incoming packet).

Original issue reported on code.google.com by pkwar...@gmail.com on 30 Jun 2009 at 5:50

GoogleCodeExporter commented 9 years ago
Here is a patch for this issue against the current trunk.

Original comment by pkwar...@gmail.com on 25 Jan 2010 at 9:06

Attachments:

GoogleCodeExporter commented 9 years ago
Here is a cumulative patch of all memory leaks and memory allocation issues. 
This fixes issues 13, 14, and 18.

Original comment by pkwar...@gmail.com on 3 Sep 2010 at 4:55

Attachments: