Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
GoogleCodeExporter opened this issue Nov 3, 2015 · 2 comments
Open

Comments

@GoogleCodeExporter
Copy link

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
Copy link
Author

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
Copy link
Author

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:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant