Merge pull request #4943 from elliefm/v311/fatals-abort

configurable abort on fatal errors
cyrusimap · Aug 19, 2024 · 352f9a0 · 352f9a0
2 parents 96f5868 + ed5d2b9
commit 352f9a0
Show file tree

Hide file tree

Showing 39 changed files with 195 additions and 12 deletions.
diff --git a/backup/backupd.c b/backup/backupd.c
@@ -173,6 +173,9 @@ EXPORTED void fatal(const char* s, int code)
         prot_flush(backupd_out);
     }
     syslog(LOG_ERR, "Fatal error: %s", s);
+
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     shut_down(code);
 }
 

diff --git a/backup/ctl_backups.c b/backup/ctl_backups.c
@@ -72,6 +72,9 @@ EXPORTED void fatal(const char *error, int code)
 {
     fprintf(stderr, "fatal error: %s\n", error);
     cyrus_done();
+
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     exit(code);
 }
 

diff --git a/backup/cyr_backup.c b/backup/cyr_backup.c
@@ -75,6 +75,9 @@ EXPORTED void fatal(const char *error, int code)
 {
     fprintf(stderr, "fatal error: %s\n", error);
     cyrus_done();
+
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     exit(code);
 }
 

diff --git a/backup/restore.c b/backup/restore.c
@@ -68,6 +68,9 @@ EXPORTED void fatal(const char *s, int code)
 {
     fprintf(stderr, "Fatal error: %s\n", s);
     syslog(LOG_ERR, "Fatal error: %s", s);
+
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     exit(code);
 }
 

diff --git a/cassandane/Cassandane/Cyrus/Simple.pm b/cassandane/Cassandane/Cyrus/Simple.pm
@@ -302,4 +302,63 @@ EOF
     $self->assert_str_equals("test\r\n", $res->{1}{binary});
 }
 
+sub test_fatals_abort_enabled
+    :NoStartInstances
+{
+    my ($self) = @_;
+
+    $self->{instance}->{config}->set(
+        'fatals_abort' => 'yes',
+        'prometheus_enabled' => 'no',
+    );
+    $self->_start_instances();
+
+    my $basedir = $self->{instance}->get_basedir();
+
+    # run `promstatsd -1` without having set up for prometheus, which should
+    # produce a "Prometheus metrics are not being tracked..." fatal error
+    eval {
+        $self->{instance}->run_command({ cyrus => 1 }, 'promstatsd', '-1');
+    };
+    my $e = $@;
+    $self->assert_not_null($e);
+    $self->assert_matches(qr{promstatsd pid \d+\) terminated by signal 6},
+                          $e->{'-text'});
+
+    my @cores = $self->{instance}->find_cores();
+    if (@cores) {
+        # if we dumped core, there'd better only be one core file
+        $self->assert_num_equals(1, scalar @cores);
+
+        # don't barf on it existing during shutdown
+        unlink $cores[0];
+    }
+}
+
+sub test_fatals_abort_disabled
+    :NoStartInstances
+{
+    my ($self) = @_;
+
+    $self->{instance}->{config}->set(
+        'fatals_abort' => 'no',
+        'prometheus_enabled' => 'no',
+    );
+    $self->_start_instances();
+
+    my $basedir = $self->{instance}->get_basedir();
+
+    # run `promstatsd -1` without having set up for prometheus, which should
+    # produce a "Prometheus metrics are not being tracked..." fatal error
+    eval {
+        $self->{instance}->run_command({ cyrus => 1 }, 'promstatsd', '-1');
+    };
+    my $e = $@;
+    $self->assert_not_null($e);
+    $self->assert_matches(qr{promstatsd pid \d+\) exited with code 78},
+                          $e->{'-text'});
+
+    # post-test sanity checks will complain for us if a core was left behind
+}
+
 1;
diff --git a/changes/next/fatals-abort b/changes/next/fatals-abort
@@ -0,0 +1,18 @@
+Description:
+
+Adds an option for fatal errors to abort and produce a core dump.
+
+
+Config changes:
+
+fatals_abort
+
+
+Upgrade instructions:
+
+None
+
+
+GitHub issue:
+
+None
diff --git a/imap/calalarmd.c b/imap/calalarmd.c
@@ -76,6 +76,8 @@ EXPORTED void fatal(const char *msg, int err)
 
     cyrus_done();
 
+    if (err != EX_PROTOCOL && config_fatals_abort) abort();
+
     exit(err);
 }
 

diff --git a/imap/cli_fatal.c b/imap/cli_fatal.c
@@ -44,6 +44,7 @@
 
 #include <stdlib.h>
 #include <stdio.h>
+#include <sysexits.h>
 
 #include "global.h"
 #include "xmalloc.h"
@@ -60,5 +61,8 @@ EXPORTED void fatal(const char *message, int code)
     recurse_code = code;
     fprintf(stderr, "fatal error: %s\n", message);
     cyrus_done();
+
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     exit(code);
 }
diff --git a/imap/ctl_conversationsdb.c b/imap/ctl_conversationsdb.c
@@ -1038,6 +1038,8 @@ EXPORTED void fatal(const char* s, int code)
 {
     fprintf(stderr, "ctl_conversationsdb: %s\n", s);
     cyrus_done();
+
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     exit(code);
 }
-
diff --git a/imap/cvt_xlist_specialuse.c b/imap/cvt_xlist_specialuse.c
@@ -87,6 +87,9 @@ EXPORTED void fatal(const char *s, int code)
 {
     fprintf(stderr, "Fatal error: %s\n", s);
     syslog(LOG_ERR, "Fatal error: %s", s);
+
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     exit(code);
 }
 

diff --git a/imap/deliver.c b/imap/deliver.c
@@ -127,6 +127,9 @@ EXPORTED void fatal(const char* s, int code)
     prot_printf(deliver_out,"421 4.3.0 deliver: %s\r\n", s);
     prot_flush(deliver_out);
     cyrus_done();
+
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     exit(code);
 }
 

diff --git a/imap/fud.c b/imap/fud.c
@@ -481,6 +481,7 @@ EXPORTED void fatal(const char* s, int code)
     recurse_code = code;
     syslog(LOG_ERR, "Fatal error: %s", s);
 
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     shut_down(code);
 }
-
diff --git a/imap/httpd.c b/imap/httpd.c
@@ -1235,6 +1235,9 @@ EXPORTED void fatal(const char* s, int code)
     }
 
     syslog(LOG_ERR, "%s%s", fatal, s);
+
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     shut_down(code);
 }
 

diff --git a/imap/idled.c b/imap/idled.c
@@ -113,6 +113,8 @@ EXPORTED void fatal(const char *msg, int err)
 
     cyrus_done();
 
+    if (err != EX_PROTOCOL && config_fatals_abort) abort();
+
     exit(err);
 }
 

diff --git a/imap/imapd.c b/imap/imapd.c
@@ -1442,6 +1442,9 @@ EXPORTED void fatal(const char *s, int code)
     }
 
     syslog(LOG_ERR, "Fatal error: %s", s);
+
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     shut_down(code);
 }
 

diff --git a/imap/lmtpd.c b/imap/lmtpd.c
@@ -1034,10 +1034,10 @@ EXPORTED void fatal(const char* s, int code)
 
     syslog(LOG_ERR, "FATAL: %s", s);
 
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     /* shouldn't return */
     shut_down(code);
-
-    exit(code);
 }
 
 /*

diff --git a/imap/message_test.c b/imap/message_test.c
@@ -311,6 +311,9 @@ EXPORTED void fatal(const char* s, int code)
 {
     fprintf(stderr, "message_test: %s\n", s);
     cyrus_done();
+
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     exit(code);
 }
 

diff --git a/imap/mupdate.c b/imap/mupdate.c
@@ -617,10 +617,10 @@ EXPORTED void fatal(const char *s, int code)
     else recurse_code = code;
 
     syslog(LOG_ERR, "%s", s);
-    shut_down(code);
 
-    /* NOTREACHED */
-    exit(code); /* shut up GCC */
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
+    shut_down(code);
 }
 
 #define CHECKNEWLINE(c, ch) do { if ((ch) == '\r') (ch)=prot_getc((c)->pin); \

diff --git a/imap/nntpd.c b/imap/nntpd.c
@@ -646,6 +646,9 @@ EXPORTED void fatal(const char* s, int code)
     }
     if (stage) append_removestage(stage);
     syslog(LOG_ERR, "Fatal error: %s", s);
+
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     shut_down(code);
 }
 

diff --git a/imap/pop3d.c b/imap/pop3d.c
@@ -674,6 +674,9 @@ EXPORTED void fatal(const char* s, int code)
         prot_flush(popd_out);
     }
     syslog(LOG_ERR, "Fatal error: %s", s);
+
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     shut_down(code);
 }
 

diff --git a/imap/promstatsd.c b/imap/promstatsd.c
@@ -111,6 +111,8 @@ EXPORTED void fatal(const char *msg, int err)
     syslog(LOG_CRIT, "%s", msg);
     syslog(LOG_NOTICE, "exiting");
 
+    if (err != EX_PROTOCOL && config_fatals_abort) abort();
+
     shut_down(err);
 }
 

diff --git a/imap/search_test.c b/imap/search_test.c
@@ -332,6 +332,9 @@ EXPORTED void fatal(const char* s, int code)
 {
     fprintf(stderr, "search_test: %s\n", s);
     cyrus_done();
+
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     exit(code);
 }
 

diff --git a/imap/smmapd.c b/imap/smmapd.c
@@ -166,7 +166,8 @@ EXPORTED void fatal(const char* s, int code)
     }
     recurse_code = code;
     syslog(LOG_ERR, "Fatal error: %s", s);
-    abort();
+
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
 
     shut_down(code);
 }

diff --git a/imap/sync_client.c b/imap/sync_client.c
@@ -138,6 +138,9 @@ EXPORTED void fatal(const char *s, int code)
 {
     fprintf(stderr, "Fatal error: %s\n", s);
     syslog(LOG_ERR, "Fatal error: %s", s);
+
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     exit(code);
 }
 

diff --git a/imap/sync_reset.c b/imap/sync_reset.c
@@ -116,6 +116,9 @@ static int usage(const char *name)
 EXPORTED void fatal(const char* s, int code)
 {
     fprintf(stderr, "sync_reset: %s\n", s);
+
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
+
     exit(code);
 }
 

diff --git a/imap/sync_server.c b/imap/sync_server.c
@@ -465,6 +465,7 @@ EXPORTED void fatal(const char* s, int code)
         prot_flush(sync_out);
     }
     syslog(LOG_ERR, "Fatal error: %s", s);
+    if (code != EX_PROTOCOL && config_fatals_abort) abort();
     shut_down(code);
 }
 

diff --git a/lib/assert.c b/lib/assert.c
@@ -47,12 +47,15 @@
 #include "xmalloc.h"
 #include "assert.h"
 
+extern int config_fatals_abort;
+
 EXPORTED void
 assertionfailed(const char *file, int line, const char *expr)
 {
     char buf[1024];
 
     snprintf(buf, sizeof(buf), "Internal error: assertion failed: %s: %d%s%s",
             file, line, expr ? ": " : "", expr ? expr : "");
+    if (config_fatals_abort) abort();
     fatal(buf, EX_SOFTWARE);
 }
diff --git a/lib/imapoptions b/lib/imapoptions
@@ -998,6 +998,14 @@ Blank lines and lines beginning with ``#'' are ignored.
    For backward compatibility, if no unit is specified, seconds is
    assumed. */
 
+{ "fatals_abort", 0, SWITCH, "UNRELEASED" }
+/* If enabled, when fatal errors are detected, Cyrus will call abort()
+   to produce a core dump, unless the error was due to a misbehaving
+   client.  This is useful if you have the ability to debug a core dump.
+.PP
+   If not enabled (the default), the process will exit normally with
+   an error code. */
+
 { "flushseenstate", 1, SWITCH, "2.5.0", "2.5.0" }
 /* Deprecated. No longer used. */
 

diff --git a/lib/libconfig.c b/lib/libconfig.c
@@ -91,6 +91,7 @@ EXPORTED int config_qosmarking;
 EXPORTED int config_debug;
 EXPORTED toggle_debug_cb config_toggle_debug_cb = NULL;
 EXPORTED int config_debug_slowio = 0;
+EXPORTED int config_fatals_abort = 0;
 
 static int config_loaded;
 
@@ -663,6 +664,7 @@ EXPORTED void config_reset(void)
     config_debug = 0;
     config_toggle_debug_cb = NULL;
     config_debug_slowio = 0;
+    config_fatals_abort = 0;
 
     /* reset all the options */
     for (opt = IMAPOPT_ZERO; opt < IMAPOPT_LAST; opt++) {
@@ -893,6 +895,9 @@ EXPORTED void config_read(const char *alt_config, const int config_need_data)
 
     /* do we want artificially-slow I/O ops */
     config_debug_slowio = config_getswitch(IMAPOPT_DEBUG_SLOWIO);
+
+    /* do we want to abort() on fatal errors */
+    config_fatals_abort = config_getswitch(IMAPOPT_FATALS_ABORT);
 }
 
 #define GROWSIZE 4096

diff --git a/lib/libconfig.h b/lib/libconfig.h
@@ -103,6 +103,7 @@ extern unsigned config_maxword;
 extern int config_qosmarking;
 extern int config_debug;
 extern int config_debug_slowio;
+extern int config_fatals_abort;
 
 /* for toggling config_debug and its behaviours at runtime */
 typedef void (*toggle_debug_cb)(void);