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

Add a patch for scapy to fix fd leak issue in AsyncSniffer #20415

Merged
merged 1 commit into from
Oct 30, 2024

Conversation

bingwang-ms
Copy link
Contributor

Why I did it

This PR is to add a patch to fix potential fd leak issue in AsyncSniffer in scapy python library.
There are two fd leak scenarios.

  1. When starting worker thread _run, if an interface is down, an OSError is thrown, and the sockets that have been created will be leaked as it never got a chance to be closed.
  2. When stopping the worker thread, same error can happen when calling close. The sockets not closed will be leaked.

Test code:

from scapy.all import *
import time


sniffing_iface = ["PortChannel101", "PortChannel102", "PortChannel106", "PortChannel1024"]
sniffstring = "udp"

sniffer = None

def process_packet(packet):
    print("Got a packet {}".format(len(packet)))

while True:
    sniffer = AsyncSniffer(iface=sniffing_iface, filter=sniffstring, prn=process_packet, store=False)
    try:
        sniffer.start()
        RETRY = 30
        while RETRY >= 0 and not hasattr(sniffer, 'stop_cb'):
            time.sleep(0.1)
            RETRY -= 1
        if RETRY < 0:
            print("Sniffer failed to start")
        else:
            print("Sniffer started")
        try:
            sniffer.stop()
            time.sleep(1)
            print("Sniffer stopped")
        except:
            pass
    except:
        time.sleep(1)
        pass

Run above code while toggling portchannels on the device. The opened fd is increasing.

Work item tracking
  • Microsoft ADO 29735068:

How I did it

Catch OSError when creating sockets, and catch any exception when closing socket to ensure all sockets are closed.

How to verify it

Verified by the testing code above. No fd leak happened.

Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
  • 202305
  • 202311
  • 202405

Tested branch (Please provide the tested image version)

The change is tested on 202311 and 202405 branches.

Description for the changelog

fix potential fd leak issue in AsyncSniffer in scapy python library.

Link to config_db schema for YANG module changes

No config change.

A picture of a cute animal (not mandatory but encouraged)

@bingwang-ms
Copy link
Contributor Author

/azpw ms_conflict

1 similar comment
@bingwang-ms
Copy link
Contributor Author

/azpw ms_conflict

@yxieca yxieca merged commit bf1bcb2 into master Oct 30, 2024
23 checks passed
@yxieca yxieca deleted the fix_fd_leak_in_scapy_asyn_sniff branch October 30, 2024 11:42
mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Nov 6, 2024
…#20415)

Why I did it
This PR is to add a patch to fix potential fd leak issue in AsyncSniffer in scapy python library.
There are two fd leak scenarios.

When starting worker thread _run, if an interface is down, an OSError is thrown, and the sockets that have been created will be leaked as it never got a chance to be closed.
When stopping the worker thread, same error can happen when calling close. The sockets not closed will be leaked.

How I did it
Catch OSError when creating sockets, and catch any exception when closing socket to ensure all sockets are closed.

How to verify it
Verified by the testing code above. No fd leak happened.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #20719

mssonicbld pushed a commit to mssonicbld/sonic-buildimage that referenced this pull request Nov 6, 2024
…#20415)

Why I did it
This PR is to add a patch to fix potential fd leak issue in AsyncSniffer in scapy python library.
There are two fd leak scenarios.

When starting worker thread _run, if an interface is down, an OSError is thrown, and the sockets that have been created will be leaked as it never got a chance to be closed.
When stopping the worker thread, same error can happen when calling close. The sockets not closed will be leaked.

How I did it
Catch OSError when creating sockets, and catch any exception when closing socket to ensure all sockets are closed.

How to verify it
Verified by the testing code above. No fd leak happened.
@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202311: #20722

mssonicbld pushed a commit that referenced this pull request Nov 7, 2024
Why I did it
This PR is to add a patch to fix potential fd leak issue in AsyncSniffer in scapy python library.
There are two fd leak scenarios.

When starting worker thread _run, if an interface is down, an OSError is thrown, and the sockets that have been created will be leaked as it never got a chance to be closed.
When stopping the worker thread, same error can happen when calling close. The sockets not closed will be leaked.

How I did it
Catch OSError when creating sockets, and catch any exception when closing socket to ensure all sockets are closed.

How to verify it
Verified by the testing code above. No fd leak happened.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants