From 2113ac4cbb12b815804e8873d761cade9ddf49b9 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Fri, 31 Mar 2017 20:35:51 -0400
Subject: [PATCH] Don't use bgw_main even to specify in-core bgworker
entrypoints.
On EXEC_BACKEND builds, this can fail if ASLR is in use.
Backpatch to 9.5. On master, completely remove the bgw_main field
completely, since there is no situation in which it is safe for an
EXEC_BACKEND build. On 9.6 and 9.5, leave the field intact to avoid
breaking things for third-party code that doesn't care about working
under EXEC_BACKEND. Prior to 9.5, there are no in-core bgworker
entrypoints.
Petr Jelinek, reviewed by me.
Discussion: http://postgr.es/m/09d8ad33-4287-a09b-a77f-77f8761adb5e@2ndquadrant.com
---
doc/src/sgml/bgworker.sgml | 36 +++-------
src/backend/access/transam/parallel.c | 6 +-
src/backend/postmaster/bgworker.c | 81 ++++++++++++++--------
src/backend/replication/logical/launcher.c | 6 +-
src/include/access/parallel.h | 2 +
src/include/postmaster/bgworker.h | 5 +-
src/test/modules/test_shm_mq/setup.c | 1 -
src/test/modules/worker_spi/worker_spi.c | 4 +-
8 files changed, 75 insertions(+), 66 deletions(-)
diff --git a/doc/src/sgml/bgworker.sgml b/doc/src/sgml/bgworker.sgml
index 07f9f1081c3..b4223230819 100644
--- a/doc/src/sgml/bgworker.sgml
+++ b/doc/src/sgml/bgworker.sgml
@@ -54,9 +54,8 @@ typedef struct BackgroundWorker
int bgw_flags;
BgWorkerStartTime bgw_start_time;
int bgw_restart_time; /* in seconds, or BGW_NEVER_RESTART */
- bgworker_main_type bgw_main;
- char bgw_library_name[BGW_MAXLEN]; /* only if bgw_main is NULL */
- char bgw_function_name[BGW_MAXLEN]; /* only if bgw_main is NULL */
+ char bgw_library_name[BGW_MAXLEN];
+ char bgw_function_name[BGW_MAXLEN];
Datum bgw_main_arg;
char bgw_extra[BGW_EXTRALEN];
int bgw_notify_pid;
@@ -130,27 +129,13 @@ typedef struct BackgroundWorker
process in case of a crash.
</para>
- <para>
- <structfield>bgw_main</structfield> is a pointer to the function to run when
- the process is started. This field can only safely be used to launch
- functions within the core server, because shared libraries may be loaded
- at different starting addresses in different backend processes. This will
- happen on all platforms when the library is loaded using any mechanism
- other than <xref linkend="guc-shared-preload-libraries">. Even when that
- mechanism is used, address space layout variations will still occur on
- Windows, and when <literal>EXEC_BACKEND</> is used. Therefore, most users
- of this API should set this field to NULL. If it is non-NULL, it takes
- precedence over <structfield>bgw_library_name</> and
- <structfield>bgw_function_name</>.
- </para>
-
<para>
<structfield>bgw_library_name</structfield> is the name of a library in
which the initial entry point for the background worker should be sought.
The named library will be dynamically loaded by the worker process and
<structfield>bgw_function_name</structfield> will be used to identify the
- function to be called. If loading a function from the core code,
- <structfield>bgw_main</> should be set instead.
+ function to be called. If loading a function from the core code, this must
+ be set to "postgres".
</para>
<para>
@@ -161,13 +146,10 @@ typedef struct BackgroundWorker
<para>
<structfield>bgw_main_arg</structfield> is the <type>Datum</> argument
- to the background worker main function. Regardless of whether that
- function is specified via <structfield>bgw_main</> or via the combination
- of <function>bgw_library_name</> and <function>bgw_function_name</>,
- this main function should take a single argument of type <type>Datum</>
- and return <type>void</>. <structfield>bgw_main_arg</structfield> will be
- passed as the argument. In addition, the global variable
- <literal>MyBgworkerEntry</literal>
+ to the background worker main function. This main function should take a
+ single argument of type <type>Datum</> and return <type>void</>.
+ <structfield>bgw_main_arg</structfield> will be passed as the argument.
+ In addition, the global variable <literal>MyBgworkerEntry</literal>
points to a copy of the <structname>BackgroundWorker</structname> structure
passed at registration time; the worker may find it helpful to examine
this structure.
@@ -215,7 +197,7 @@ typedef struct BackgroundWorker
<para>
Signals are initially blocked when control reaches the
- <structfield>bgw_main</> function, and must be unblocked by it; this is to
+ background worker's main function, and must be unblocked by it; this is to
allow the process to customize its signal handlers, if necessary.
Signals can be unblocked in the new process by calling
<function>BackgroundWorkerUnblockSignals</> and blocked by calling
diff --git a/src/backend/access/transam/parallel.c b/src/backend/access/transam/parallel.c
index 3e0ee87e203..b3d3853fbc2 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -110,7 +110,6 @@ static dlist_head pcxt_list = DLIST_STATIC_INIT(pcxt_list);
/* Private functions. */
static void HandleParallelMessage(ParallelContext *pcxt, int i, StringInfo msg);
static void ParallelExtensionTrampoline(dsm_segment *seg, shm_toc *toc);
-static void ParallelWorkerMain(Datum main_arg);
static void WaitForParallelWorkersToExit(ParallelContext *pcxt);
@@ -458,7 +457,8 @@ LaunchParallelWorkers(ParallelContext *pcxt)
| BGWORKER_CLASS_PARALLEL;
worker.bgw_start_time = BgWorkerStart_ConsistentState;
worker.bgw_restart_time = BGW_NEVER_RESTART;
- worker.bgw_main = ParallelWorkerMain;
+ sprintf(worker.bgw_library_name, "postgres");
+ sprintf(worker.bgw_function_name, "ParallelWorkerMain");
worker.bgw_main_arg = UInt32GetDatum(dsm_segment_handle(pcxt->seg));
worker.bgw_notify_pid = MyProcPid;
memset(&worker.bgw_extra, 0, BGW_EXTRALEN);
@@ -931,7 +931,7 @@ AtEOXact_Parallel(bool isCommit)
/*
* Main entrypoint for parallel workers.
*/
-static void
+void
ParallelWorkerMain(Datum main_arg)
{
dsm_segment *seg;
diff --git a/src/backend/postmaster/bgworker.c b/src/backend/postmaster/bgworker.c
index 10e0f88b0de..0823317b780 100644
--- a/src/backend/postmaster/bgworker.c
+++ b/src/backend/postmaster/bgworker.c
@@ -15,12 +15,14 @@
#include <unistd.h>
#include "libpq/pqsignal.h"
+#include "access/parallel.h"
#include "miscadmin.h"
#include "pgstat.h"
#include "port/atomics.h"
#include "postmaster/bgworker_internals.h"
#include "postmaster/postmaster.h"
#include "replication/logicallauncher.h"
+#include "replication/logicalworker.h"
#include "storage/dsm.h"
#include "storage/ipc.h"
#include "storage/latch.h"
@@ -109,14 +111,26 @@ struct BackgroundWorkerHandle
static BackgroundWorkerArray *BackgroundWorkerData;
/*
- * List of workers that are allowed to be started outside of
- * shared_preload_libraries.
+ * List of internal background workers. These are used for mapping the
+ * function name to actual function when building with EXEC_BACKEND and also
+ * to allow these to be loaded outside of shared_preload_libraries.
*/
-static const bgworker_main_type InternalBGWorkers[] = {
- ApplyLauncherMain,
- NULL
+typedef struct InternalBGWorkerMain
+{
+ char *bgw_function_name;
+ bgworker_main_type bgw_main;
+} InternalBGWorkerMain;
+
+static const InternalBGWorkerMain InternalBGWorkers[] = {
+ {"ParallelWorkerMain", ParallelWorkerMain},
+ {"ApplyLauncherMain", ApplyLauncherMain},
+ {"ApplyWorkerMain", ApplyWorkerMain},
+ /* Dummy entry marking end of the array. */
+ {NULL, NULL}
};
+static bgworker_main_type GetInternalBgWorkerMain(BackgroundWorker *worker);
+
/*
* Calculate shared memory needed.
*/
@@ -341,7 +355,6 @@ BackgroundWorkerStateChange(void)
rw->rw_worker.bgw_flags = slot->worker.bgw_flags;
rw->rw_worker.bgw_start_time = slot->worker.bgw_start_time;
rw->rw_worker.bgw_restart_time = slot->worker.bgw_restart_time;
- rw->rw_worker.bgw_main = slot->worker.bgw_main;
rw->rw_worker.bgw_main_arg = slot->worker.bgw_main_arg;
memcpy(rw->rw_worker.bgw_extra, slot->worker.bgw_extra, BGW_EXTRALEN);
@@ -763,17 +776,14 @@ StartBackgroundWorker(void)
}
/*
- * If bgw_main is set, we use that value as the initial entrypoint.
- * However, if the library containing the entrypoint wasn't loaded at
- * postmaster startup time, passing it as a direct function pointer is not
- * possible. To work around that, we allow callers for whom a function
- * pointer is not available to pass a library name (which will be loaded,
- * if necessary) and a function name (which will be looked up in the named
- * library).
+ * For internal workers set the entry point to known function address.
+ * Otherwise use the entry point specified by library name (which will
+ * be loaded, if necessary) and a function name (which will be looked up
+ * in the named library).
*/
- if (worker->bgw_main != NULL)
- entrypt = worker->bgw_main;
- else
+ entrypt = GetInternalBgWorkerMain(worker);
+
+ if (entrypt == NULL)
entrypt = (bgworker_main_type)
load_external_function(worker->bgw_library_name,
worker->bgw_function_name,
@@ -806,23 +816,13 @@ RegisterBackgroundWorker(BackgroundWorker *worker)
{
RegisteredBgWorker *rw;
static int numworkers = 0;
- bool internal = false;
- int i;
if (!IsUnderPostmaster)
ereport(DEBUG1,
(errmsg("registering background worker \"%s\"", worker->bgw_name)));
- for (i = 0; InternalBGWorkers[i]; i++)
- {
- if (worker->bgw_main == InternalBGWorkers[i])
- {
- internal = true;
- break;
- }
- }
-
- if (!process_shared_preload_libraries_in_progress && !internal)
+ if (!process_shared_preload_libraries_in_progress &&
+ GetInternalBgWorkerMain(worker) == NULL)
{
if (!IsUnderPostmaster)
ereport(LOG,
@@ -1152,3 +1152,28 @@ TerminateBackgroundWorker(BackgroundWorkerHandle *handle)
if (signal_postmaster)
SendPostmasterSignal(PMSIGNAL_BACKGROUND_WORKER_CHANGE);
}
+
+/*
+ * Search the known internal worker array and return its main function
+ * pointer if found.
+ *
+ * Returns NULL if not known internal worker.
+ */
+static bgworker_main_type
+GetInternalBgWorkerMain(BackgroundWorker *worker)
+{
+ int i;
+
+ /* Internal workers always have to use postgres as library name. */
+ if (strncmp(worker->bgw_library_name, "postgres", BGW_MAXLEN) != 0)
+ return NULL;
+
+ for (i = 0; InternalBGWorkers[i].bgw_function_name; i++)
+ {
+ if (strncmp(InternalBGWorkers[i].bgw_function_name,
+ worker->bgw_function_name, BGW_MAXLEN) == 0)
+ return InternalBGWorkers[i].bgw_main;
+ }
+
+ return NULL;
+}
diff --git a/src/backend/replication/logical/launcher.c b/src/backend/replication/logical/launcher.c
index 255b22597b6..fecff936c07 100644
--- a/src/backend/replication/logical/launcher.c
+++ b/src/backend/replication/logical/launcher.c
@@ -295,7 +295,8 @@ logicalrep_worker_launch(Oid dbid, Oid subid, const char *subname, Oid userid,
bgw.bgw_flags = BGWORKER_SHMEM_ACCESS |
BGWORKER_BACKEND_DATABASE_CONNECTION;
bgw.bgw_start_time = BgWorkerStart_RecoveryFinished;
- bgw.bgw_main = ApplyWorkerMain;
+ snprintf(bgw.bgw_library_name, BGW_MAXLEN, "postgres");
+ snprintf(bgw.bgw_function_name, BGW_MAXLEN, "ApplyWorkerMain");
if (OidIsValid(relid))
snprintf(bgw.bgw_name, BGW_MAXLEN,
"logical replication worker for subscription %u sync %u", subid, relid);
@@ -553,7 +554,8 @@ ApplyLauncherRegister(void)
bgw.bgw_flags = BGWORKER_SHMEM_ACCESS |
BGWORKER_BACKEND_DATABASE_CONNECTION;
bgw.bgw_start_time = BgWorkerStart_RecoveryFinished;
- bgw.bgw_main = ApplyLauncherMain;
+ snprintf(bgw.bgw_library_name, BGW_MAXLEN, "postgres");
+ snprintf(bgw.bgw_function_name, BGW_MAXLEN, "ApplyLauncherMain");
snprintf(bgw.bgw_name, BGW_MAXLEN,
"logical replication launcher");
bgw.bgw_restart_time = 5;
diff --git a/src/include/access/parallel.h b/src/include/access/parallel.h
index 96ce4803dad..5065a3830cf 100644
--- a/src/include/access/parallel.h
+++ b/src/include/access/parallel.h
@@ -67,4 +67,6 @@ extern void AtEOXact_Parallel(bool isCommit);
extern void AtEOSubXact_Parallel(bool isCommit, SubTransactionId mySubId);
extern void ParallelWorkerReportLastRecEnd(XLogRecPtr last_xlog_end);
+extern void ParallelWorkerMain(Datum main_arg);
+
#endif /* PARALLEL_H */
diff --git a/src/include/postmaster/bgworker.h b/src/include/postmaster/bgworker.h
index 128e92cea1d..51a5978ea87 100644
--- a/src/include/postmaster/bgworker.h
+++ b/src/include/postmaster/bgworker.h
@@ -91,9 +91,8 @@ typedef struct BackgroundWorker
int bgw_flags;
BgWorkerStartTime bgw_start_time;
int bgw_restart_time; /* in seconds, or BGW_NEVER_RESTART */
- bgworker_main_type bgw_main;
- char bgw_library_name[BGW_MAXLEN]; /* only if bgw_main is NULL */
- char bgw_function_name[BGW_MAXLEN]; /* only if bgw_main is NULL */
+ char bgw_library_name[BGW_MAXLEN];
+ char bgw_function_name[BGW_MAXLEN];
Datum bgw_main_arg;
char bgw_extra[BGW_EXTRALEN];
pid_t bgw_notify_pid; /* SIGUSR1 this backend on start/stop */
diff --git a/src/test/modules/test_shm_mq/setup.c b/src/test/modules/test_shm_mq/setup.c
index e3c024ecb4c..319a67f49aa 100644
--- a/src/test/modules/test_shm_mq/setup.c
+++ b/src/test/modules/test_shm_mq/setup.c
@@ -216,7 +216,6 @@ setup_background_workers(int nworkers, dsm_segment *seg)
worker.bgw_flags = BGWORKER_SHMEM_ACCESS;
worker.bgw_start_time = BgWorkerStart_ConsistentState;
worker.bgw_restart_time = BGW_NEVER_RESTART;
- worker.bgw_main = NULL; /* new worker might not have library loaded */
sprintf(worker.bgw_library_name, "test_shm_mq");
sprintf(worker.bgw_function_name, "test_shm_mq_main");
snprintf(worker.bgw_name, BGW_MAXLEN, "test_shm_mq");
diff --git a/src/test/modules/worker_spi/worker_spi.c b/src/test/modules/worker_spi/worker_spi.c
index 72ab8464e12..421ec76ba36 100644
--- a/src/test/modules/worker_spi/worker_spi.c
+++ b/src/test/modules/worker_spi/worker_spi.c
@@ -347,7 +347,8 @@ _PG_init(void)
BGWORKER_BACKEND_DATABASE_CONNECTION;
worker.bgw_start_time = BgWorkerStart_RecoveryFinished;
worker.bgw_restart_time = BGW_NEVER_RESTART;
- worker.bgw_main = worker_spi_main;
+ sprintf(worker.bgw_library_name, "worker_spi");
+ sprintf(worker.bgw_function_name, "worker_spi_main");
worker.bgw_notify_pid = 0;
/*
@@ -378,7 +379,6 @@ worker_spi_launch(PG_FUNCTION_ARGS)
BGWORKER_BACKEND_DATABASE_CONNECTION;
worker.bgw_start_time = BgWorkerStart_RecoveryFinished;
worker.bgw_restart_time = BGW_NEVER_RESTART;
- worker.bgw_main = NULL; /* new worker might not have library loaded */
sprintf(worker.bgw_library_name, "worker_spi");
sprintf(worker.bgw_function_name, "worker_spi_main");
snprintf(worker.bgw_name, BGW_MAXLEN, "worker %d", i);
--
GitLab