]> granicus.if.org Git - postgresql/commitdiff
Don't use bgw_main even to specify in-core bgworker entrypoints.
authorRobert Haas <rhaas@postgresql.org>
Sat, 1 Apr 2017 00:35:51 +0000 (20:35 -0400)
committerRobert Haas <rhaas@postgresql.org>
Sat, 1 Apr 2017 00:43:32 +0000 (20:43 -0400)
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
src/backend/access/transam/parallel.c
src/backend/postmaster/bgworker.c
src/backend/replication/logical/launcher.c
src/include/access/parallel.h
src/include/postmaster/bgworker.h
src/test/modules/test_shm_mq/setup.c
src/test/modules/worker_spi/worker_spi.c

index 07f9f1081c3e5cf19431baedeb4a5278f28edf7f..b42232308199fd0ba6dc21d5e1c83e1d03b68cfa 100644 (file)
@@ -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
index 3e0ee87e203618ebc496ba0f392ae00d97fe751c..b3d3853fbc202662bb9c369900a61f6e790a2013 100644 (file)
@@ -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;
index 10e0f88b0de49f666fda1d0c78b30aae4a092de8..0823317b78080a3563a5036fe915d0dc15a6ebe9 100644 (file)
 #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;
+}
index 255b22597b64c5afcf7593817e46ae60d89e4460..fecff936c07c8ecf0bf9359d3667cb9c52c2a2f9 100644 (file)
@@ -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;
index 96ce4803dadcbdf60bed3a01769329d397d88173..5065a3830cf5700d7c0617b7b88936b0809748ab 100644 (file)
@@ -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 */
index 128e92cea1d3954005f00d55ceebc2fecb97dfab..51a5978ea872281b1ea79173c48a00629a9f6abf 100644 (file)
@@ -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 */
index e3c024ecb4cf0c8653d8c03873d6916b6e3d0c92..319a67f49aa3ad9882ef222598cc684230026596 100644 (file)
@@ -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");
index 72ab8464e12ea0bb6458305463d0038935d6ab16..421ec76ba36ba332677a45c98d2802e15e7771b7 100644 (file)
@@ -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);