From fae0d1855c74aa13574943370ffbe79f768bdb01 Mon Sep 17 00:00:00 2001
From: Nathan Bossart <nathan@postgresql.org>
Date: Fri, 17 Nov 2023 21:38:18 -0600
Subject: [PATCH v2 1/3] Check that MyProcPid == getpid() in all signal
 handlers.

In commit 97550c0711, we added a similar check to the SIGTERM
handler for the startup process.  This commit adds this check to
all signal handlers installed with pqsignal().  This is done by
using a wrapper function that performs the check before calling the
actual handler.

The hope is that this will offer more general protection against
grandchildren processes inadvertently modifying shared memory due
to inherited signal handlers.  Another potential follow-up
improvement is to use this wrapper handler function to restore
errno instead of relying on each individual handler function to do
so.

This commit makes the changes in commit 97550c0711 obsolete but
leaves reverting it for a follow-up commit.

Reviewed-by: ???
Discussion: ???
---
 src/port/pqsignal.c | 74 +++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 72 insertions(+), 2 deletions(-)

diff --git a/src/port/pqsignal.c b/src/port/pqsignal.c
index 83d876db6c..0baf8eed11 100644
--- a/src/port/pqsignal.c
+++ b/src/port/pqsignal.c
@@ -28,23 +28,87 @@
 #include "c.h"
 
 #include <signal.h>
+#ifndef FRONTEND
+#include <unistd.h>
+#endif
 
 #ifndef FRONTEND
 #include "libpq/pqsignal.h"
+#include "miscadmin.h"
+#endif
+
+#ifdef PG_SIGNAL_COUNT			/* Windows */
+#define PG_NSIG (PG_SIGNAL_COUNT)
+#elif defined(NSIG)
+#define PG_NSIG (NSIG)
+#else
+#define PG_NSIG (64)			/* XXX: wild guess */
+#endif
+
+static volatile pqsigfunc pqsignal_handlers[PG_NSIG];
+
+/*
+ * Except when called with SIG_IGN or SIG_DFL, pqsignal() sets up this function
+ * as the handler for all signals.  This wrapper handler function checks that
+ * it is called within a process that the server knows about, and not a
+ * grandchild process forked by system(3), etc.  This check ensures that such
+ * grandchildren processes do not modify shared memory, which could be
+ * detrimental.  If the check succeeds, the function originally provided to
+ * pqsignal() is called.  Otherwise, the default signal handler is installed
+ * and then called.
+ */
+static void
+wrapper_handler(SIGNAL_ARGS)
+{
+#ifndef FRONTEND
+	Assert(MyProcPid);
+
+	if (unlikely(MyProcPid != (int) getpid()))
+	{
+		pqsignal(postgres_signal_arg, SIG_DFL);
+		raise(postgres_signal_arg);
+		return;
+	}
 #endif
 
+	(*pqsignal_handlers[postgres_signal_arg]) (postgres_signal_arg);
+}
+
 /*
  * Set up a signal handler, with SA_RESTART, for signal "signo"
  *
  * Returns the previous handler.
+ *
+ * NB: If called within a signal handler, race conditions may lead to bogus
+ * return values.  You should either avoid calling this within signal handlers
+ * or ignore the return value.
+ *
+ * XXX: Since no in-tree callers use the return value, and there is little
+ * reason to do so, it would be nice if we could convert this to a void
+ * function instead of providing potentially-bogus return values.
+ * Unfortunately, that requires modifying the pqsignal() in legacy-pqsignal.c,
+ * which in turn requires an SONAME bump, which is probably not worth it.
  */
 pqsigfunc
 pqsignal(int signo, pqsigfunc func)
 {
+	pqsigfunc	orig_func = pqsignal_handlers[signo];	/* assumed atomic */
 #if !(defined(WIN32) && defined(FRONTEND))
 	struct sigaction act,
 				oact;
+#else
+	pqsigfunc	ret;
+#endif
+
+	Assert(signo < PG_NSIG);
 
+	if (func != SIG_IGN && func != SIG_DFL)
+	{
+		pqsignal_handlers[signo] = func;	/* assumed atomic */
+		func = wrapper_handler;
+	}
+
+#if !(defined(WIN32) && defined(FRONTEND))
 	act.sa_handler = func;
 	sigemptyset(&act.sa_mask);
 	act.sa_flags = SA_RESTART;
@@ -54,9 +118,15 @@ pqsignal(int signo, pqsigfunc func)
 #endif
 	if (sigaction(signo, &act, &oact) < 0)
 		return SIG_ERR;
-	return oact.sa_handler;
+	else if (oact.sa_handler == wrapper_handler)
+		return orig_func;
+	else
+		return oact.sa_handler;
 #else
 	/* Forward to Windows native signal system. */
-	return signal(signo, func);
+	if ((ret = signal(signo, func)) == wrapper_handler)
+		return orig_func;
+	else
+		return ret;
 #endif
 }
-- 
2.25.1