commit 6a183f8: [Feature] Add race condition protection against hs_helper restarts

[Vsevolod Stakhov]

Author: Vsevolod Stakhov
Date: 2021-05-12 17:42:55 +0100
URL: https://github.com/rspamd/rspamd/commit/6a183f820fd4db594a7a9730aa084a8216827fd4 (HEAD -> master)

[Feature] Add race condition protection against hs_helper restarts

---
 src/hs_helper.c          | 37 +++++++++++++++++++++++++++++++++----
 src/libserver/re_cache.c |  7 ++++---
 2 files changed, 37 insertions(+), 7 deletions(-)

diff --git a/src/hs_helper.c b/src/hs_helper.c
index f4f00cf05..cba2ca230 100644
--- a/src/hs_helper.c
+++ b/src/hs_helper.c
@@ -123,6 +123,7 @@ rspamd_hs_helper_cleanup_dir (struct hs_helper_ctx *ctx, gboolean forced)
 	gint rc;
 	gchar *pattern;
 	gboolean ret = TRUE;
+	pid_t our_pid = getpid ();
 
 	if (stat (ctx->hs_dir, &st) == -1) {
 		msg_err ("cannot stat path %s, %s",
@@ -160,10 +161,38 @@ rspamd_hs_helper_cleanup_dir (struct hs_helper_ctx *ctx, gboolean forced)
 	rspamd_snprintf (pattern, len, "%s%c%s", ctx->hs_dir, G_DIR_SEPARATOR, "*.hs.new");
 	if ((rc = glob (pattern, 0, NULL, &globbuf)) == 0) {
 		for (i = 0; i < globbuf.gl_pathc; i++) {
-			if (unlink (globbuf.gl_pathv[i]) == -1) {
-				msg_err ("cannot unlink %s: %s", globbuf.gl_pathv[i],
-						strerror (errno));
-				ret = FALSE;
+			/* Check if we have a pid in the filename */
+			const gchar *end_num = globbuf.gl_pathv[i] +
+					strlen (globbuf.gl_pathv[i]) - (sizeof (".hs.new") - 1);
+			const gchar *p = end_num - 1;
+			pid_t foreign_pid = -1;
+
+			while (p > globbuf.gl_pathv[i]) {
+				if (g_ascii_isdigit (*p)) {
+					p --;
+				}
+				else {
+					p ++;
+					break;
+				}
+			}
+
+			gulong ul;
+			if (p < end_num && rspamd_strtoul (p, end_num - p, &ul)) {
+				foreign_pid = ul;
+			}
+
+			/*
+			 * Remove only files that was left by us or some non-existing process
+			 * There could be another race condition but it would just leave
+			 * extra files which is relatively innocent?
+			 */
+			if (foreign_pid == -1 || foreign_pid == our_pid || kill (foreign_pid, 0) == -1) {
+				if (unlink(globbuf.gl_pathv[i]) == -1) {
+					msg_err ("cannot unlink %s: %s", globbuf.gl_pathv[i],
+							strerror(errno));
+					ret = FALSE;
+				}
 			}
 		}
 	}
diff --git a/src/libserver/re_cache.c b/src/libserver/re_cache.c
index b3326bcff..1d2f6522f 100644
--- a/src/libserver/re_cache.c
+++ b/src/libserver/re_cache.c
@@ -1894,6 +1894,7 @@ rspamd_re_cache_compile_timer_cb (EV_P_ ev_timer *w, int revents )
 	struct iovec iov[7];
 	struct rspamd_re_cache *cache;
 	GError *err;
+	pid_t our_pid = getpid ();
 
 	cache = cbdata->cache;
 
@@ -1946,8 +1947,8 @@ rspamd_re_cache_compile_timer_cb (EV_P_ ev_timer *w, int revents )
 		return;
 	}
 
-	rspamd_snprintf (path, sizeof (path), "%s%c%s.hs.new", cbdata->cache_dir,
-			G_DIR_SEPARATOR, re_class->hash);
+	rspamd_snprintf (path, sizeof (path), "%s%c%s.%P.hs.new", cbdata->cache_dir,
+			G_DIR_SEPARATOR, re_class->hash, our_pid);
 	fd = open (path, O_CREAT|O_TRUNC|O_EXCL|O_WRONLY, 00600);
 
 	if (fd == -1) {
@@ -2185,7 +2186,7 @@ rspamd_re_cache_compile_timer_cb (EV_P_ ev_timer *w, int revents )
 		g_free (hs_flags);
 
 		/* Now rename temporary file to the new .hs file */
-		rspamd_snprintf (npath, sizeof (path), "%s%c%s.hs", cbdata->cache_dir,
+		rspamd_snprintf (npath, sizeof (npath), "%s%c%s.hs", cbdata->cache_dir,
 				G_DIR_SEPARATOR, re_class->hash);
 
 		if (rename (path, npath) == -1) {