summaryrefslogtreecommitdiff
path: root/src
diff options
context:
space:
mode:
Diffstat (limited to 'src')
-rw-r--r--src/lib/err/backtrace.c42
-rw-r--r--src/lib/err/backtrace.h1
-rw-r--r--src/lib/err/torerr.c11
-rw-r--r--src/lib/process/process_unix.c9
-rw-r--r--src/lib/process/process_win32.c18
-rw-r--r--src/test/test_process_slow.c30
6 files changed, 90 insertions, 21 deletions
diff --git a/src/lib/err/backtrace.c b/src/lib/err/backtrace.c
index 2408a68513..8bc7e6965c 100644
--- a/src/lib/err/backtrace.c
+++ b/src/lib/err/backtrace.c
@@ -69,10 +69,10 @@
// Redundant with util.h, but doing it here so we can avoid that dependency.
#define raw_free free
-#ifdef USE_BACKTRACE
/** Version of Tor to report in backtrace messages. */
static char bt_version[128] = "";
+#ifdef USE_BACKTRACE
/** Largest stack depth to try to dump. */
#define MAX_DEPTH 256
/** Static allocation of stack to dump. This is static so we avoid stack
@@ -128,7 +128,7 @@ log_backtrace_impl(int severity, log_domain_mask_t domain, const char *msg,
depth = backtrace(cb_buf, MAX_DEPTH);
symbols = backtrace_symbols(cb_buf, (int)depth);
- logger(severity, domain, "%s. Stack trace:", msg);
+ logger(severity, domain, "%s: %s. Stack trace:", bt_version, msg);
if (!symbols) {
/* LCOV_EXCL_START -- we can't provoke this. */
logger(severity, domain, " Unable to generate backtrace.");
@@ -198,13 +198,10 @@ static int trap_signals[] = { SIGSEGV, SIGILL, SIGFPE, SIGBUS, SIGSYS,
/** Install signal handlers as needed so that when we crash, we produce a
* useful stack trace. Return 0 on success, -errno on failure. */
static int
-install_bt_handler(const char *software)
+install_bt_handler(void)
{
int i, rv=0;
- strncpy(bt_version, software, sizeof(bt_version) - 1);
- bt_version[sizeof(bt_version) - 1] = 0;
-
struct sigaction sa;
memset(&sa, 0, sizeof(sa));
@@ -263,13 +260,13 @@ void
log_backtrace_impl(int severity, log_domain_mask_t domain, const char *msg,
tor_log_fn logger)
{
- logger(severity, domain, "%s. (Stack trace not available)", msg);
+ logger(severity, domain, "%s: %s. (Stack trace not available)",
+ bt_version, msg);
}
static int
-install_bt_handler(const char *software)
+install_bt_handler(void)
{
- (void) software;
return 0;
}
@@ -284,6 +281,14 @@ dump_stack_symbols_to_error_fds(void)
}
#endif /* defined(NO_BACKTRACE_IMPL) */
+/** Return the tor version used for error messages on crashes.
+ * Signal-safe: returns a pointer to a static array. */
+const char *
+get_tor_backtrace_version(void)
+{
+ return bt_version;
+}
+
/** Set up code to handle generating error messages on crashes. */
int
configure_backtrace_handler(const char *tor_version)
@@ -291,10 +296,25 @@ configure_backtrace_handler(const char *tor_version)
char version[128] = "Tor\0";
if (tor_version) {
- snprintf(version, sizeof(version), "Tor %s", tor_version);
+ int snp_rv = 0;
+ /* We can't use strlcat() here, because it is defined in
+ * string/compat_string.h on some platforms, and string uses torerr. */
+ snp_rv = snprintf(version, sizeof(version), "Tor %s", tor_version);
+ /* It's safe to call raw_assert() here, because raw_assert() does not
+ * call configure_backtrace_handler(). */
+ raw_assert(snp_rv < (int)sizeof(version));
+ raw_assert(snp_rv >= 0);
}
- return install_bt_handler(version);
+ char *str_rv = NULL;
+ /* We can't use strlcpy() here, see the note about strlcat() above. */
+ str_rv = strncpy(bt_version, version, sizeof(bt_version) - 1);
+ /* We must terminate bt_version, then raw_assert(), because raw_assert()
+ * uses bt_version. */
+ bt_version[sizeof(bt_version) - 1] = 0;
+ raw_assert(str_rv == bt_version);
+
+ return install_bt_handler();
}
/** Perform end-of-process cleanup for code that generates error messages on
diff --git a/src/lib/err/backtrace.h b/src/lib/err/backtrace.h
index dcd22cfef2..7e09a0a5a7 100644
--- a/src/lib/err/backtrace.h
+++ b/src/lib/err/backtrace.h
@@ -24,6 +24,7 @@ void log_backtrace_impl(int severity, log_domain_mask_t domain,
int configure_backtrace_handler(const char *tor_version);
void clean_up_backtrace_handler(void);
void dump_stack_symbols_to_error_fds(void);
+const char *get_tor_backtrace_version(void);
#define log_backtrace(sev, dom, msg) \
log_backtrace_impl((sev), (dom), (msg), tor_log)
diff --git a/src/lib/err/torerr.c b/src/lib/err/torerr.c
index 21b28a5f69..b7e32a3e20 100644
--- a/src/lib/err/torerr.c
+++ b/src/lib/err/torerr.c
@@ -198,14 +198,21 @@ tor_raw_assertion_failed_msg_(const char *file, int line, const char *expr,
{
char linebuf[16];
format_dec_number_sigsafe(line, linebuf, sizeof(linebuf));
- tor_log_err_sigsafe("INTERNAL ERROR: Raw assertion failed at ",
- file, ":", linebuf, ": ", expr, NULL);
+ tor_log_err_sigsafe("INTERNAL ERROR: Raw assertion failed in ",
+ get_tor_backtrace_version(), " at ",
+ file, ":", linebuf, ": ", expr, "\n", NULL);
if (msg) {
tor_log_err_sigsafe_write(msg);
tor_log_err_sigsafe_write("\n");
}
dump_stack_symbols_to_error_fds();
+
+ /* Some platforms (macOS, maybe others?) can swallow the last write before an
+ * abort. This issue is probably caused by a race condition between write
+ * buffer cache flushing, and process termination. So we write an extra
+ * newline, to make sure that the message always gets through. */
+ tor_log_err_sigsafe_write("\n");
}
/**
diff --git a/src/lib/process/process_unix.c b/src/lib/process/process_unix.c
index 17ade87463..ef81ec9bf9 100644
--- a/src/lib/process/process_unix.c
+++ b/src/lib/process/process_unix.c
@@ -253,22 +253,15 @@ process_unix_exec(process_t *process)
process_environment_t *env = process_get_environment(process);
/* Call the requested program. */
- retval = execve(argv[0], argv, env->unixoid_environment_block);
+ execve(argv[0], argv, env->unixoid_environment_block);
/* If we made it here it is because execve failed :-( */
- if (-1 == retval)
- fprintf(stderr, "Call to execve() failed: %s", strerror(errno));
-
tor_free(argv);
process_environment_free(env);
- tor_assert_unreached();
-
error:
- /* LCOV_EXCL_START */
fprintf(stderr, "Error from child process: %s", strerror(errno));
_exit(1);
- /* LCOV_EXCL_STOP */
}
/* We are in the parent process. */
diff --git a/src/lib/process/process_win32.c b/src/lib/process/process_win32.c
index 624333d4a3..7e4082ad13 100644
--- a/src/lib/process/process_win32.c
+++ b/src/lib/process/process_win32.c
@@ -234,6 +234,24 @@ process_win32_exec(process_t *process)
CloseHandle(stdin_pipe_read);
CloseHandle(stdin_pipe_write);
+ /* In the Unix backend, we do not get an error in the Tor process when a
+ * child process fails to spawn its target executable since we need to
+ * first do the fork() call in the Tor process and then the child process
+ * is responsible for doing the call to execve().
+ *
+ * This means that the user of the process_exec() API must check for
+ * whether it returns PROCESS_STATUS_ERROR, which will rarely happen on
+ * Unix, but will happen for error cases on Windows where it does not
+ * happen on Unix. For example: when the target executable does not exist
+ * on the file system.
+ *
+ * To have somewhat feature compatibility between the Unix and the Windows
+ * backend, we here notify the process_t owner that the process have exited
+ * (even though it never managed to run) to ensure that the exit callback
+ * is executed.
+ */
+ process_notify_event_exit(process, 0);
+
return PROCESS_STATUS_ERROR;
}
diff --git a/src/test/test_process_slow.c b/src/test/test_process_slow.c
index 91252c725d..f311e8b293 100644
--- a/src/test/test_process_slow.c
+++ b/src/test/test_process_slow.c
@@ -328,8 +328,38 @@ test_callbacks_terminate(void *arg)
process_free(process);
}
+static void
+test_nonexistent_executable(void *arg)
+{
+ (void)arg;
+
+ /* Process callback data. */
+ process_data_t *process_data = process_data_new();
+
+ /* Setup our process. */
+ process_t *process = process_new("binary-does-not-exist");
+ process_set_data(process, process_data);
+ process_set_exit_callback(process, process_exit_callback);
+
+ /* Run our process. */
+ process_exec(process);
+
+ /* Start our main loop. */
+ run_main_loop(process_data);
+
+ /* Ensure that the exit callback was actually called even though the binary
+ * did not exist.
+ */
+ tt_assert(process_data->did_exit);
+
+ done:
+ process_data_free(process_data);
+ process_free(process);
+}
+
struct testcase_t slow_process_tests[] = {
{ "callbacks", test_callbacks, 0, NULL, NULL },
{ "callbacks_terminate", test_callbacks_terminate, 0, NULL, NULL },
+ { "nonexistent_executable", test_nonexistent_executable, 0, NULL, NULL },
END_OF_TESTCASES
};
7apu2bq3rhoylwmucxizk54afw5jyda7pho4j5zdcqpwkufke7zdzwad.onion