diff --git a/scapy/sendrecv.py b/scapy/sendrecv.py index ae1d05b158e..3f98e727c60 100644 --- a/scapy/sendrecv.py +++ b/scapy/sendrecv.py @@ -1053,6 +1053,23 @@ def srp1flood(x, # type: _PacketIterable # SNIFF METHODS +def _offline_pcap_reader(fname, flt, quiet): + # type: (str, Optional[str], bool) -> PcapReader + """Build a PcapReader for one sniff() offline source. + + Without a filter the file is read directly. With a filter the packets are + prefiltered through a tcpdump subprocess; that process is attached to the + reader so its close() reaps it. Reading it through a bare file descriptor + used to leak the process as a zombie (#4512). + """ + if flt is None: + return PcapReader(fname) + proc = tcpdump(fname, args=["-w", "-"], flt=flt, getproc=True, quiet=quiet) + reader = PcapReader(proc.stdout) + reader.subproc = proc + return reader + + class AsyncSniffer(object): """ Sniff packets and return a list of packets. @@ -1194,24 +1211,16 @@ def _run(self, if isinstance(offline, list) and \ all(isinstance(elt, str) for elt in offline): # List of files - sniff_sockets.update((PcapReader( # type: ignore - fname if flt is None else - tcpdump(fname, - args=["-w", "-"], - flt=flt, - getfd=True, - quiet=quiet) - ), fname) for fname in offline) + sniff_sockets.update( + (_offline_pcap_reader(fname, flt, quiet), fname) # type: ignore + for fname in offline + ) elif isinstance(offline, dict): # Dict of files - sniff_sockets.update((PcapReader( # type: ignore - fname if flt is None else - tcpdump(fname, - args=["-w", "-"], - flt=flt, - getfd=True, - quiet=quiet) - ), label) for fname, label in offline.items()) + sniff_sockets.update( + (_offline_pcap_reader(fname, flt, quiet), label) # type: ignore + for fname, label in offline.items() + ) elif isinstance(offline, (Packet, PacketList, list)): # Iterables (list of packets, PacketList..) offline = IterSocket(offline) diff --git a/scapy/utils.py b/scapy/utils.py index facd90d8803..43c120d7c4a 100644 --- a/scapy/utils.py +++ b/scapy/utils.py @@ -1410,6 +1410,10 @@ class RawPcapReader(metaclass=PcapReader_metaclass): nonblocking_socket = True PacketMetadata = collections.namedtuple("PacketMetadata", ["sec", "usec", "wirelen", "caplen"]) # noqa: E501 + # A helper subprocess (e.g. the tcpdump prefilter that sniff() spawns for + # an offline capture with a filter) whose lifetime is bound to this reader. + # It is reaped in close() so it does not linger as a zombie (#4512). + subproc = None # type: Optional[subprocess.Popen[bytes]] def __init__(self, filename, fdesc=None, magic=None): # type: ignore # type: (str, _ByteStream, bytes) -> None @@ -1529,6 +1533,13 @@ def close(self): if isinstance(self.f, gzip.GzipFile): self.f.fileobj.close() # type: ignore self.f.close() + if self.subproc is not None: + # Reap the prefilter subprocess. The read pipe is already closed + # above, so a still-running tcpdump gets a SIGTERM and we then + # wait() to avoid a zombie; an already-finished one is just reaped. + self.subproc.terminate() + self.subproc.wait() + self.subproc = None def __exit__(self, exc_type, exc_value, tracback): # type: (Optional[Any], Optional[Any], Optional[Any]) -> None diff --git a/test/regression.uts b/test/regression.uts index af284e224a6..cb27ff904fa 100644 --- a/test/regression.uts +++ b/test/regression.uts @@ -2464,6 +2464,37 @@ assert all(UDP in p for p in l) l = sniff(offline=IP()/UDP(sport=(10000, 10001)), filter="tcp") assert len(l) == 0 += Offline sniff() with a filter reaps its tcpdump process (#4512) +~ tcpdump libpcap +import gc, warnings + +fdesc, filename = tempfile.mkstemp() +os.close(fdesc) +wrpcap(filename, [IP()/TCP()] * 20) + +def _leaked_subprocess_warnings(**kwargs): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + packets = sniff(offline=filename, filter="tcp", **kwargs) + gc.collect() + leaked = [str(w.message) for w in caught + if issubclass(w.category, ResourceWarning) + and "subprocess" in str(w.message)] + return packets, leaked + +# Reading the whole file: the prefilter process must be waited on in close(). +packets, leaked = _leaked_subprocess_warnings() +assert len(packets) == 20 +assert not leaked, leaked + +# Stopping early leaves tcpdump running; close() must terminate and reap it +# rather than block or leave a zombie behind. +packets, leaked = _leaked_subprocess_warnings(count=3) +assert len(packets) == 3 +assert not leaked, leaked + +os.unlink(filename) + = Check offline sniff() with Packets, tcpdump and a bad filter ~ tcpdump libpcap