fprintf(3) should not be used by signal handlers

[Sashan]

On most OS platforms fprintf() et.al. are considered to be signal unsafe. I've noticed the signal handlers in tcpdump call fprintf(). The patch below defers call to fprintf() triggered by signal to pcap_loop callback.

--- a/tcpdump.c 2017-10-05 06:03:58.598362638 -0700
+++ b/tcpdump.c 2017-10-05 05:58:49.245478018 -0700
@@ -165,8 +165,8 @@
 static char *zflag = NULL;             /* compress each savefile using a specified command (like gzip or bzip2) */
 static int immediate_mode;
 
-static int infodelay;
 static int infoprint;
+static int gotalarm;
 
 char *program_name;
 
@@ -1961,7 +1961,9 @@
                setvbuf(stderr, NULL, _IONBF, 0);
 #elif defined(HAVE_ALARM)
                (void)setsignal(SIGALRM, verbose_stats_dump);
-               alarm(1);
+               alarm(1);
+               /* signal alarm to get the first iteration ASAP */
+               gotalarm = 1;
 #endif
        }
 
@@ -2274,8 +2276,6 @@
 
        ++packets_captured;
 
-       ++infodelay;
-
        dump_info = (struct dump_info *)user;
 
        /*
@@ -2464,9 +2464,16 @@
                pcap_dump_flush(dump_info->p);
 #endif
 
-       --infodelay;
-       if (infoprint)
+       if (infoprint) {
                info(0);
+               infoprint = 0;
+       }
+
+       if (gotalarm) {
+               fprintf(stderr, "Got %u\r", packets_captured);
+               gotalarm = 0;
+               alarm(1);
+       }
 }
 
 static void
@@ -2474,8 +2481,6 @@
 {
        ++packets_captured;
 
-       ++infodelay;
-
        pcap_dump(user, h, sp);
 #ifdef HAVE_PCAP_DUMP_FLUSH
        if (Uflag)
@@ -2482,9 +2487,16 @@
                pcap_dump_flush((pcap_dumper_t *)user);
 #endif
 
-       --infodelay;
-       if (infoprint)
+       if (infoprint) {
                info(0);
+               infoprint = 0;
+       }
+
+       if (gotalarm) {
+               fprintf(stderr, "Got %u\r", packets_captured);
+               gotalarm = 0;
+               alarm(1);
+       }
 }
 
 static void
@@ -2492,13 +2504,12 @@
 {
        ++packets_captured;
 
-       ++infodelay;
-
        pretty_print_packet((netdissect_options *)user, h, sp, packets_captured);
 
-       --infodelay;
-       if (infoprint)
+       if (infoprint) {
                info(0);
+               infoprint = 0;
+       }
 }
 
 #ifdef _WIN32
@@ -2531,10 +2542,7 @@
 #ifdef SIGNAL_REQ_INFO
 RETSIGTYPE requestinfo(int signo _U_)
 {
-       if (infodelay)
-               ++infoprint;
-       else
-               info(0);
+       infoprint = 1;
 }
 #endif
 
@@ -2545,15 +2553,12 @@
 void CALLBACK verbose_stats_dump (UINT timer_id _U_, UINT msg _U_, DWORD_PTR arg _U_,
                                  DWORD_PTR dw1 _U_, DWORD_PTR dw2 _U_)
 {
-       if (infodelay == 0)
-               fprintf(stderr, "Got %u\r", packets_captured);
+       gotalarm = 1;
 }
 #elif defined(HAVE_ALARM)
 static void verbose_stats_dump(int sig _U_)
 {
-       if (infodelay == 0)
-               fprintf(stderr, "Got %u\r", packets_captured);
-       alarm(1);
+       gotalarm = 1;
 }
 #endif

Patch above is against tcpdump-4.9.2. It looks like the signal-safety here prevents tcpdump -w dump.pkt -v to work on Solaris. After applying patch above the issue gets fixed.

I understand proposed solution has some drawback. If there are no packets captured, then nothing gets printed. The ultimate fix calls for much more complex change.

thanks and
regards
sashan