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

Reading from stdin while dumping to file causes crash #155

Open
guyharris opened this issue Apr 16, 2013 · 10 comments
Open

Reading from stdin while dumping to file causes crash #155

guyharris opened this issue Apr 16, 2013 · 10 comments

Comments

@guyharris
Copy link
Member

Converted from SourceForge issue 3602279, submitted by toreanderson

When attempting to reading from stdin and dumping the result to a capture file, the process crashes almost instantly. An easy way to reproduce this, is:

computer$ nc -l 12345 | tcpdump -r- -v -wfoo.pcap
reading from file -, link-type EN10MB (Ethernet)
tcpdump: pcap_loop: error reading dump file: Interrupted system call
write: Broken pipe

If I omit the "-wroo.pcap" option, it works just fine, decoded traffic is printed to stdout just as expected and no crashes are experienced.

The version used is the one included in Fedora 18: tcpdump-4.3.0-2.fc18.x86_64.

The PCAP stream coming in on 12345/tcp to the nc process is a tcpdump running on another machine like so: "tcpdump -i eth1.3900 -s0 -w- -U ip6 | nc computer 12345".

@infrastation
Copy link
Member

This reproduces on the current master branch on a Linux host.

@guyharris
Copy link
Member Author

To quote a comment in tcpdump.c:

            /*
             * When capturing to a file, "-v" means tcpdump should,
             * every 10 secodns, "v"erbosely report the number of
             * packets captured.
             */

In most cases, -v isn't useful when reading from a file and writing to a file, rather than when capturing from a device and writing to a file; however, there's probably no harm in supporting it - if you don't want that output, don't specify -v.

In this case, it might be more useful, as you are, in effect, capturing from the other machine.

Therefore, we need to do something with EINTR when reading packets. Obviously, if it was interrupted by SIGINT (^C), we should quit, but if it was interrupted by SIGALRM (the timer for -v when writing to a file), we should just keep reading.

@guyharris
Copy link
Member Author

This will require libpcap changes - merely setting the SA_RESTART flag for SIGALRM means that, instead of getting "Interrupted system call", we get "truncated dump file; tried to read 300 captured bytes, only got 170". At least when reading from a pipe, we need to keep reading bytes until we get an EOF.

@guyharris
Copy link
Member Author

See also the-tcpdump-group/libpcap#1258.

@fenner
Copy link
Contributor

fenner commented Aug 24, 2020

Naively, since we're only setting the callback when using -w, can't this be implemented in dump_packet() instead? If enough time has passed, then call print_packets_captured(), and don't set the itimer at all?

@infrastation
Copy link
Member

If there is a difficult to fix reason for once per second counter update not to work when reading from a file, let's enable the counter for live captures only and document the behaviour. This would resolve the corner case crash; if anybody wants to make it perfect later, they can propose a patch.

@fenner
Copy link
Contributor

fenner commented Aug 24, 2020

I naively suggested:

Naively, ... can't this be implemented in dump_packet() instead?

Obviously there's a significant difference in that, if you get 1000 packets in the first 0.9 seconds, and then no more, the counter will never update. (The use case that I'm interested in is sflowtool, which converts sflow-format captured packets to pcap, so libpcap will block regularly.)

infrastation said:

let's enable the counter for live captures only and document the behaviour

This is obviously the best short-term approach.

@infrastation
Copy link
Member

Do you think a separate thread around print_packets_captured() would be easier to implement? Would it still be portable enough?

@fenner
Copy link
Contributor

fenner commented Aug 25, 2020

Do you think a separate thread around print_packets_captured() would be easier to implement? Would it still be portable enough?

I've always tried to avoid threads in portable code, but I don't have an educated opinion here, more just a mystical one :-)

infrastation added a commit that referenced this issue Aug 29, 2020
As explained in GH #155, when tcpdump is given -r, -w and -v and it
takes long enough to read from the input file (because it is stdin
connected through network or a pipe to stdout of another tcpdump doing
a live capture), pcap_loop() will error before long. One of the ways to
reproduce the fault is as follows:

$ tcpdump -i eno1 -w - | tcpdump -r - -w /tmp/tmp.pcap -v
tcpdump: listening on eno1, link-type EN10MB (Ethernet), snapshot length 262144 bytes
reading from file -, link-type EN10MB (Ethernet), snapshot length 262144
tcpdump: pcap_loop: error reading dump file: Interrupted system call

Skip the verbose_stats_dump() timer setup in this specific corner case
for the time being and document it.
@infrastation
Copy link
Member

The master branch of tcpdump now has a documented safeguard with comments as discussed above. @guyharris, is signal handling in libpcap you described earlier still an important enough problem to state/document it elsewhere and maybe to work on a solution?

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

No branches or pull requests

3 participants