From 165dcacd984e57dc6dac714128a3192e80b1e62a Mon Sep 17 00:00:00 2001
From: Robin Sommer <robin@corelight.com>
Date: Mon, 24 Aug 2020 10:26:58 +0000
Subject: [PATCH] Make set_processing_status() signal-safe.

Closes #574.
---
 src/util.cc                                   | 58 +++++++++++--------
 .../Baseline/core.proc-status-file/error      |  4 ++
 testing/btest/core/proc-status-file.zeek      |  3 +
 3 files changed, 42 insertions(+), 23 deletions(-)
 create mode 100644 testing/btest/Baseline/core.proc-status-file/error

diff --git a/src/util.cc b/src/util.cc
index 36f3d19139..41edbea655 100644
--- a/src/util.cc
+++ b/src/util.cc
@@ -963,31 +963,13 @@ void terminate_processing()
 
 void set_processing_status(const char* status, const char* reason)
 	{
+	// This function can be called from a signal context, so we have to
+	// make sure to only call reentrant & async-signal-safe functions,
+	// and to restore errno afterwards.
+
 	if ( ! proc_status_file )
 		return;
 
-	// This function can be called from a signal context, so we have to
-	// make sure to only call reentrant functions and to restore errno
-	// afterwards.
-
-	int old_errno = errno;
-
-	int fd = open(proc_status_file, O_CREAT | O_WRONLY | O_TRUNC, 0700);
-
-	if ( fd < 0 )
-		{
-		char buf[256];
-		zeek_strerror_r(errno, buf, sizeof(buf));
-		if ( zeek::reporter )
-			zeek::reporter->Error("Failed to open process status file '%s': %s",
-			                      proc_status_file, buf);
-		else
-			fprintf(stderr, "Failed to open process status file '%s': %s\n",
-			        proc_status_file, buf);
-		errno = old_errno;
-		return;
-		}
-
 	auto write_str = [](int fd, const char* s)
 		{
 		int len = strlen(s);
@@ -1005,11 +987,41 @@ void set_processing_status(const char* status, const char* reason)
 			}
 		};
 
+	auto report_error_with_errno = [&](const char* msg)
+		{
+		// strerror_r() is not async-signal-safe, hence we don't do
+		// the translation from errno to string.
+		auto errno_str = std::to_string(errno);
+		write_str(2, msg);
+		write_str(2, " '");
+		write_str(2, proc_status_file);
+		write_str(2, "': ");
+		write_str(2, errno_str.c_str());
+		write_str(2, " [");
+		write_str(2, status);
+		write_str(2, "]\n");
+		};
+
+	int old_errno = errno;
+
+	int fd = open(proc_status_file, O_CREAT | O_WRONLY | O_TRUNC, 0700);
+	if ( fd < 0 )
+		{
+		report_error_with_errno("Failed to open process status file");
+		errno = old_errno;
+		return;
+		}
+
 	write_str(fd, status);
 	write_str(fd, " [");
 	write_str(fd, reason);
 	write_str(fd, "]\n");
-	safe_close(fd);
+
+	if ( close(fd) < 0 && errno != EINTR )
+			{
+			report_error_with_errno("Failed to close process status file");
+			abort(); // same as safe_close()
+			}
 
 	errno = old_errno;
 	}
diff --git a/testing/btest/Baseline/core.proc-status-file/error b/testing/btest/Baseline/core.proc-status-file/error
new file mode 100644
index 0000000000..6ed0944bdb
--- /dev/null
+++ b/testing/btest/Baseline/core.proc-status-file/error
@@ -0,0 +1,4 @@
+Failed to open process status file '/cannot/write/to/this/file': XX [INITIALIZING]
+Failed to open process status file '/cannot/write/to/this/file': XX [RUNNING]
+Failed to open process status file '/cannot/write/to/this/file': XX [TERMINATED]
+Failed to open process status file '/cannot/write/to/this/file': XX [TERMINATING]
diff --git a/testing/btest/core/proc-status-file.zeek b/testing/btest/core/proc-status-file.zeek
index ff69a86116..1cd8f7e40c 100644
--- a/testing/btest/core/proc-status-file.zeek
+++ b/testing/btest/core/proc-status-file.zeek
@@ -1,2 +1,5 @@
 # @TEST-EXEC: zeek -b -U status-file -e ''
 # @TEST-EXEC: btest-diff status-file
+
+# @TEST-EXEC: zeek -b -U /cannot/write/to/this/file -e '' 2>&1 | sed 's/: [0-9]\{1,\}/: XX/g' | sort | uniq >error
+# @TEST-EXEC: btest-diff error