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

Resource leak caused by the mismatch of pcap_create and pcap_close #1233

Closed
PromptFuzz opened this issue Nov 24, 2023 · 3 comments
Closed

Resource leak caused by the mismatch of pcap_create and pcap_close #1233

PromptFuzz opened this issue Nov 24, 2023 · 3 comments

Comments

@PromptFuzz
Copy link

Hi,
I have found that if call pcap_close() intermediatly after the pcap_create() could cause a eventfd leak.

The example code is:

    if (pcap_findalldevs(&devs, errbuf) == -1) {
     ...
    }
    // Open first device for capturing
    handle = pcap_create(devs->name, errbuf);
    //pcap_activate(handle);
    pcap_close(handle);   

My environment :
OS: CentOS 5.4
Run with non-privileged user account

Rerpoduce:
In
poc.zip.

fd_leak

I have noticed that pcap_create has announced:

The returned handle must be activated with pcap_activate(3PCAP) before packets can be captured with it;

Actually, the leak was disappeared if i called pcap_activate(handle); after the pcap_create().
But, for general use, the pcap_create and pcap_close should be match, regardless whether we use or not use the returned handler.

PromptFuzz added a commit to PromptFuzz/libpcap that referenced this issue Nov 24, 2023
See issue: the-tcpdump-group#1233
This commit initializes the cleanup_op callback of handler in pcap_create.
This avoid the resource leak caused by mismatch of resource allocation and deallocation.

Signed-off-by: promptfuzz <promptfuzz@gmail.com>
@PromptFuzz
Copy link
Author

I have provided a PR to fix this issue: #1235

guyharris added a commit to guyharris/libpcap that referenced this issue Nov 25, 2023
If we're not going to be in non-blocking mode, open the event FD in the
activate rourinte; we don't need it until we're going to activate it for
capturing.

This means we don't need to close it if a non-activated handle is being
closed.

Clean up some comments, etc. while we're at it.

Fixes issue the-tcpdump-group#1233.
@guyharris
Copy link
Member

But, for general use, the pcap_create and pcap_close should be match, regardless whether we use or not use the returned handler.

Yes, the eventfd shouldn't be opened until we need it, so it should be opened in the activate routine, not the create routine. See pull request #1236.

guyharris added a commit to guyharris/libpcap that referenced this issue Nov 25, 2023
If we're not going to be in non-blocking mode, open the event FD in the
activate rourinte; we don't need it until we're going to activate it for
capturing.

This means we don't need to close it if a non-activated handle is being
closed.

Clean up some comments, etc. while we're at it.

Fixes issue the-tcpdump-group#1233.
guyharris added a commit to guyharris/libpcap that referenced this issue Nov 25, 2023
If we're not going to be in non-blocking mode, open the event FD in the
activate rourinte; we don't need it until we're going to activate it for
capturing.

This means we don't need to close it if a non-activated handle is being
closed.

Clean up some comments, etc. while we're at it.

Fixes issue the-tcpdump-group#1233.
guyharris added a commit that referenced this issue Nov 25, 2023
If we're not going to be in non-blocking mode, open the event FD in the
activate rourinte; we don't need it until we're going to activate it for
capturing.

This means we don't need to close it if a non-activated handle is being
closed.

Clean up some comments, etc. while we're at it.

Fixes issue #1233.
@guyharris
Copy link
Member

Fixed in c62d3d3.

tenarchits pushed a commit to tenarchits/libpcap that referenced this issue Jan 22, 2024
If we're not going to be in non-blocking mode, open the event FD in the
activate rourinte; we don't need it until we're going to activate it for
capturing.

This means we don't need to close it if a non-activated handle is being
closed.

Clean up some comments, etc. while we're at it.

Fixes issue the-tcpdump-group#1233.
guyharris added a commit that referenced this issue Jan 25, 2024
If we're not going to be in non-blocking mode, open the event FD in the
activate rourinte; we don't need it until we're going to activate it for
capturing.

This means we don't need to close it if a non-activated handle is being
closed.

Clean up some comments, etc. while we're at it.

Fixes issue #1233.

(cherry picked from commit c62d3d3)
tenarchits pushed a commit to tenarchits/libpcap that referenced this issue Jan 26, 2024
If we're not going to be in non-blocking mode, open the event FD in the
activate rourinte; we don't need it until we're going to activate it for
capturing.

This means we don't need to close it if a non-activated handle is being
closed.

Clean up some comments, etc. while we're at it.

Fixes issue the-tcpdump-group#1233.
tenarchits pushed a commit to tenarchits/libpcap that referenced this issue Jan 26, 2024
If we're not going to be in non-blocking mode, open the event FD in the
activate rourinte; we don't need it until we're going to activate it for
capturing.

This means we don't need to close it if a non-activated handle is being
closed.

Clean up some comments, etc. while we're at it.

Fixes issue the-tcpdump-group#1233.
tenarchits pushed a commit to tenarchits/libpcap that referenced this issue Jan 26, 2024
If we're not going to be in non-blocking mode, open the event FD in the
activate rourinte; we don't need it until we're going to activate it for
capturing.

This means we don't need to close it if a non-activated handle is being
closed.

Clean up some comments, etc. while we're at it.

Fixes issue the-tcpdump-group#1233.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants