libpcap blocking, ignoring interrupts, after a973128

[fenner]

I've been updating my internal version of libpcap regularly. After updating this weekend, one of my tests completely hangs, and is unresponsive to SIGINT. This is just a preliminary report, in case @guyharris is working on a followup commit already - but it looks like poll() inside pcap_wait_for_frames_mmap is eating signals and not returning, and is blocking in a case that did not block before. I will gather more info but am providing this in case it rings a bell.

(This use case may be calling pcap_next() when there is no packet to be read. Ignoring error handling code, the code is:

      handle = pcap_create( dev, errbuf );
      int status = pcap_set_snaplen( handle, BUFSIZ );
      status = pcap_set_promisc( handle, 1 );
      status = pcap_set_timeout( handle, 1000 );
      status = pcap_set_immediate_mode( handle, 1 );
      status = pcap_activate( handle );
      if( pcap_setdirection( handle, PCAP_D_IN ) == -1 ){
      if( pcap_compile( handle, &fp, filterExp, 0, PCAP_NETMASK_UNKNOWN ) == -1 ){
      if( pcap_setfilter( handle, &fp ) == -1 ){
      pcap_freecode( &fp );
      int fd = pcap_get_selectable_fd( handle );

The next thing it does is register fd in our event manager, and it's plausible that this (perhaps incorrectly) causes a call to pcap_next() - but given that there's a timeout, this shouldn't be fatal, maybe just cause an unnecessary delay.

I hit ^C, which strace shows was received but ignored, and then hit ^\ to generate a SIGQUIT.

strace says:

setsockopt(16, SOL_SOCKET, SO_ATTACH_FILTER, "\6\0\0\0P\3618\t", 8) = 0
gettid()                                = 256
writev(2, [{"2020-02-03 04:18:41.660387   256"..., 56}, {"Created Pcap handler! on interfa"..., 43}, {"\n", 1}], 3) = 100
gettid()                                = 256
writev(2, [{"2020-02-03 04:18:41.660387   256"..., 56}, {"Created Pcap handler 0x9404930", 30}, {"\n", 1}], 3) = 87
fcntl64(16, F_GETFL)                    = 0x2 (flags O_RDWR)
fcntl64(16, F_SETFL, O_RDWR)            = 0
write(6, "\0", 1)                       = 1
gettid()                                = 256
writev(2, [{"2020-02-03 04:18:41.660387   256"..., 56}, {"void <...>::han"..., 58}, {"\n", 1}], 3) = 115
poll([{fd=16, events=POLLIN}, {fd=15, events=POLLIN}], 2, 1000) = 0 (Timeout)
poll([{fd=16, events=POLLIN}, {fd=15, events=POLLIN}], 2, 1000) = 0 (Timeout)
poll([{fd=16, events=POLLIN}, {fd=15, events=POLLIN}], 2, 1000) = 0 (Timeout)
poll([{fd=16, events=POLLIN}, {fd=15, events=POLLIN}], 2, 1000) = 0 (Timeout)
poll([{fd=16, events=POLLIN}, {fd=15, events=POLLIN}], 2, 1000) = 0 (Timeout)
poll([{fd=16, events=POLLIN}, {fd=15, events=POLLIN}], 2, 1000) = ? ERESTART_RESTARTBLOCK (Interrupted by signal)
--- SIGINT {si_signo=SIGINT, si_code=SI_KERNEL} ---
sigreturn({mask=[]})                    = -1 EINTR (Interrupted system call)
poll([{fd=16, events=POLLIN}, {fd=15, events=POLLIN}], 2, 1000) = ? ERESTART_RESTARTBLOCK (Interrupted by signal)
--- SIGCHLD {si_signo=SIGCHLD, si_code=CLD_EXITED, si_pid=270, si_uid=0, si_status=1, si_utime=0, si_stime=0} ---
restart_syscall(<... resuming interrupted poll ...>) = 0
poll([{fd=16, events=POLLIN}, {fd=15, events=POLLIN}], 2, 1000) = ? ERESTART_RESTARTBLOCK (Interrupted by signal)
--- SIGQUIT {si_signo=SIGQUIT, si_code=SI_KERNEL} ---

The traceback is:

#0  0xf7fd8a50 in __kernel_vsyscall ()
#1  0xf7c29d93 in __poll_nocancel () from /lib/libc.so.6
#2  0xf3ae10e8 in pcap_wait_for_frames_mmap () from target:/lib/libpcap.so.1
#3  0xf3ae1857 in pcap_read_linux_mmap_v2 () from target:/lib/libpcap.so.1
#4  0xf3aeafb6 in pcap_next () from target:/lib/libpcap.so.1

[guyharris]

So does that test program catch SIGINT?

[fenner]

So does that test program catch SIGINT?

Not explicitly, but strace shows that someone is (maybe python's KeyboardInterrupt handler). I'm ok with punting on that for now and focusing on the blocking behavior change. (I don't know if the signal problem happens with the old code because it never hangs to have time to send a SIGINT.) But I suppose the answer there is just that the program is depending on behavior that the man page says not to depend upon.

Note that the wait ... might not ... terminate even if no packets are available

So perhaps it's just worth being aware that people who just code to observed behavior might run into this when upgrading. I think the fix here is to either use pcap_setnonblock() or to not call pcap_next() this first time.

[guyharris]

So does that test program catch SIGINT?

Not explicitly, but strace shows that someone is (maybe python's KeyboardInterrupt handler). I'm ok with punting on that for now and focusing on the blocking behavior change. (I don't know if the signal problem happens with the old code because it never hangs to have time to send a SIGINT.) But I suppose the answer there is just that the program is depending on behavior that the man page says not to depend upon.

OK, I've added some flags to the capturetest test program to make it capture SIGINT and, optionally, call pcap_breakloop() in the handler, and tried it on macOS with

testprogs/capturetest -s -b -i en0

so that it captures SIGINT and calls pcap_breakloop() in the handler. Typing a ^C while it's running gave

$ testprogs/capturetest -s -b -i en0
Listening on en0
3 packets seen, 3 packets counted after pcap_dispatch returns
3 ps_recv, 0 ps_drop, 0 ps_ifdrop
^C
Broken out of loop from SIGINT handler

However, if I ran it without -b, so that it doesn't call pcap_breakloop() in the handler, ^C doesn't interrupt it:

$ testprogs/capturetest -s -i en0
Listening on en0
6 packets seen, 6 packets counted after pcap_dispatch returns
6 ps_recv, 0 ps_drop, 0 ps_ifdrop
^C2 packets seen, 2 packets counted after pcap_dispatch returns
8 ps_recv, 0 ps_drop, 0 ps_ifdrop
7 packets seen, 7 packets counted after pcap_dispatch returns
15 ps_recv, 0 ps_drop, 0 ps_ifdrop

so there was never a guarantee that, if SIGINT is caught and the handler doesn't call pcap_breakloop() on any pcap_ts on which capturing is being done, the capture will terminate. It happened to do so on Linux, at least with memory-mapped captures, but programs shouldn't have depended on that.

[guyharris]

The next thing it does is register fd in our event manager, and it's plausible that this (perhaps incorrectly) causes a call to pcap_next() - but given that there's a timeout, this shouldn't be fatal, maybe just cause an unnecessary delay.

So is this an event manager that has an event loop using select()/poll()/epoll_wait()/etc., and, if a given FD is somethingable, calls the appropriate handler, so that if the pcap_t's FD is readable, a callback that calls pcap_next() or pcap_next_ex() or pcap_dispatch() on the pcap_t in question?

If so, yes, you should call pcap_setnonblock() - you've already waited for a "read ready" event, there's no need to do a poll() to do it again.

Note also that if you have pcap_get_required_select_timeout(), you should call it, and, if it returns a non-null value, use that timeout in the event loop, and call the callback for the pcap_t if the timer expires. (Also, the return value is now a const struct timeval * in the master branch, rather than just a struct timeval * as in 4.x, so assign the value to a const struct timeval *.)

Also, the return value of pcap_get_required_select_timeout() can change from call to call; this is part of the painful process of trying, on Linux, to arrange that 1) interfaces going down do not cause a capturing error but 2) interfaces going away do.

[fenner]

The next thing it does is register fd in our event manager, and it's plausible that this (perhaps incorrectly) causes a call to pcap_next() - but given that there's a timeout, this shouldn't be fatal, maybe just cause an unnecessary delay.

So is this an event manager that has an event loop using select()/poll()/epoll_wait()/etc., and, if a given FD is somethingable, calls the appropriate handler, so that if the pcap_t's FD is readable, a callback that calls pcap_next() or pcap_next_ex() or pcap_dispatch() on the pcap_t in question?

Correct. But the problem that arose is that:
a) a minor bug in the usage of the event loop means that pcap_next() got called without the fd being marked as readable;
b) libpcap used to not block in this scenario.

If so, yes, you should call pcap_setnonblock() - you've already waited for a "read ready" event, there's no need to do a poll() to do it again.

Ok.

Note also that if you have pcap_get_required_select_timeout(), you should call it, and, if it returns a non-null value, use that timeout in the event loop, and call the callback for the pcap_t if the timer expires. (Also, the return value is now a const struct timeval * in the master branch, rather than just a struct timeval * as in 4.x, so assign the value to a const struct timeval *.)

Also, the return value of pcap_get_required_select_timeout() can change from call to call; this is part of the painful process of trying, on Linux, to arrange that 1) interfaces going down do not cause a capturing error but 2) interfaces going away do.

This is not at all straightforward to arrange given our event loop (it's more focused around "what time would you like something to happen" than "how much longer this time around the loop"), but luckily in this use case this code will know if the interface is going away and shutdown the pcap itself.

[guyharris]

The next thing it does is register fd in our event manager, and it's plausible that this (perhaps incorrectly) causes a call to pcap_next() - but given that there's a timeout, this shouldn't be fatal, maybe just cause an unnecessary delay.

So is this an event manager that has an event loop using select()/poll()/epoll_wait()/etc., and, if a given FD is somethingable, calls the appropriate handler, so that if the pcap_t's FD is readable, a callback that calls pcap_next() or pcap_next_ex() or pcap_dispatch() on the pcap_t in question?

Correct. But the problem that arose is that:
a) a minor bug in the usage of the event loop means that pcap_next() got called without the fd being marked as readable;
b) libpcap used to not block in this scenario.

With that program, on Linux, libpcap used not to ever block if the pcap_t wasn't in non-blocking mode and you called pcap_next() when the FD wasn't marked as readable? It wouldn't block if there are userland packets available, and it still should, but it should block if there aren't any userland packets available.

This is not at all straightforward to arrange given our event loop (it's more focused around "what time would you like something to happen" than "how much longer this time around the loop"), but luckily in this use case this code will know if the interface is going away and shutdown the pcap itself.

The issue is that an interface going from IFF_UP to IFF_DOWN causes a single call to poll() on the FD to return POLLERR with ENETDOWN as the error; subsequent poll() calls will block if the timeout isn't 0. Some users didn't want an error to be returned in that case - they just want to continue capturing, as the interface might come back up again - so we don't return one.

If the interface goes away completely (e.g., unplugging a USB adapter), no error is provided by the kernel, and no wakeup is even provided by the kernel. We try to detect that by, if we've gotten an ENETDOWN and haven't seen any packets since than and haven't seen the interface up since then, periodically checking whether the interface is still there.

We can do that in the loop in blocking mode, but, in non-blocking mode, we can't, so we just say "here's a timeout, try checking for packets or errors every so often". As long as you're doing that, or as long as you don't need libpcap to say "this pcap_t refers to a no-longer-extant interface, don't waste your time listening to it", you don't need to look at the timeout.

[fenner]

The next thing it does is register fd in our event manager, and it's plausible that this (perhaps incorrectly) causes a call to pcap_next() - but given that there's a timeout, this shouldn't be fatal, maybe just cause an unnecessary delay.

So is this an event manager that has an event loop using select()/poll()/epoll_wait()/etc., and, if a given FD is somethingable, calls the appropriate handler, so that if the pcap_t's FD is readable, a callback that calls pcap_next() or pcap_next_ex() or pcap_dispatch() on the pcap_t in question?

Correct. But the problem that arose is that:
a) a minor bug in the usage of the event loop means that pcap_next() got called without the fd being marked as readable;
b) libpcap used to not block in this scenario.

With that program, on Linux, libpcap used not to ever block if the pcap_t wasn't in non-blocking mode and you called pcap_next() when the FD wasn't marked as readable? It wouldn't block if there are userland packets available, and it still should, but it should block if there aren't any userland packets available.

Same test program, with libpcap 1.9.1 - strace says that poll() times out:

poll([{fd=15, events=POLLIN}], 1, 1000) = 0 (Timeout)

and debugging says that pcap_next() subsequently returns NULL. There are no packets available - this is a freshly created handle on a test interface.

(I do realize that the documented API says "you have to be able to handle blocking in this scenario", so libpcap's new behavior is not wrong - just worried that if this code made this assumption, then others might too and the new behavior might be surprising)

This is not at all straightforward to arrange given our event loop (it's more focused around "what time would you like something to happen" than "how much longer this time around the loop"), but luckily in this use case this code will know if the interface is going away and shutdown the pcap itself.

The issue is that an interface going from IFF_UP to IFF_DOWN causes a single call to poll() on the FD to return POLLERR with ENETDOWN as the error; subsequent poll() calls will block if the timeout isn't 0. Some users didn't want an error to be returned in that case - they just want to continue capturing, as the interface might come back up again - so we don't return one.

If the interface goes away completely (e.g., unplugging a USB adapter), no error is provided by the kernel, and no wakeup is even provided by the kernel. We try to detect that by, if we've gotten an ENETDOWN and haven't seen any packets since than and haven't seen the interface up since then, periodically checking whether the interface is still there.

We can do that in the loop in blocking mode, but, in non-blocking mode, we can't, so we just say "here's a timeout, try checking for packets or errors every so often". As long as you're doing that, or as long as you don't need libpcap to say "this pcap_t refers to a no-longer-extant interface, don't waste your time listening to it", you don't need to look at the timeout.

It's worth considering how this API will fit into an event system where the main loop isn't user-accessible - e.g., what if you are fitting into a system that uses libevent, and your api to the event system is just event_new()/event_add() - does the adaptor layer have to keep track of the fact that the selectable fd was readable, and so reset the timeout in the event each time? That's additional overhead per packet that might be nice to avoid - if the API can be "call something after pcap_get_required_select_timeout() whether or not any packets arrived in the meantime" that would be easier to fit into event systems that aren't just "call select".

(I suppose that this is not that hard: the callback for the absolute timeout can just consult a flag that the "fd was readable" handler sets, and if it's set, get a new timeout. This would be ok, right - if pcap_get_required_select_timeout() returned 10 seconds, and a packet arrived any time in those 10 seconds, call pcap_get_required_select_timeout() again after the 10 seconds are up? Would pcap_get_required_select_timeout() return 0 (or negative?) if the required time has passed?)

[eaglegai]

@fenner @guyharris hello, three years have passed, are there really any bugs after a973128？

[fenner]

I have no further followup.