sniff: reap the tcpdump prefilter subprocess in offline mode

scapy/sendrecv.py

@@ -1053,6 +1053,25 @@ def srp1flood(x,  # type: _PacketIterable
 # SNIFF METHODS
 
 
+def _offline_pcap_reader(source, flt, quiet):
+    # type: (Any, Optional[str], bool) -> PcapReader
+    """Build a PcapReader for one sniff() offline source.
+
+    The source is whatever tcpdump() accepts: a filename, an IterSocket over
+    packets, or an open file descriptor. Without a filter it 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(source)
+    proc = tcpdump(source, 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,44 +1213,28 @@ 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)
-                sniff_sockets[offline if flt is None else PcapReader(
-                    tcpdump(offline,
-                            args=["-w", "-"],
-                            flt=flt,
-                            getfd=True,
-                            quiet=quiet)
-                )] = offline
+                sniff_sockets[
+                    offline if flt is None
+                    else _offline_pcap_reader(offline, flt, quiet)
+                ] = offline
             else:
                 # Other (file descriptors...)
-                sniff_sockets[PcapReader(  # type: ignore
-                    offline if flt is None else
-                    tcpdump(offline,
-                            args=["-w", "-"],
-                            flt=flt,
-                            getfd=True,
-                            quiet=quiet)
-                )] = offline
+                sniff_sockets[
+                    _offline_pcap_reader(offline, flt, quiet)  # type: ignore
+                ] = offline
         if not sniff_sockets or iface is not None:
             # The _RL2 function resolves the L2socket of an iface
             _RL2 = lambda i: L2socket or resolve_iface(i).l2listen()  # type: Callable[[_GlobInterfaceType], Callable[..., SuperSocket]]  # noqa: E501

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

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