From ef1b5af82339a49564037be656a3ff657fb2a246 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Wed, 3 Aug 2016 14:48:05 -0400 Subject: [PATCH] Do not let PostmasterContext survive into background workers. We don't want postmaster child processes to contain a copy of the postmaster's PostmasterContext. That would be a waste of memory at least, and at worst a security issue, since there are copies of the semi-sensitive pg_hba and pg_ident data in there. All other child process types delete the PostmasterContext after forking, but the original coding of the background worker patch (commit da07a1e85) did not do so. It appears that the only reason for that was to avoid copying the bgworker's MyBgworkerEntry out of that context; but the couple of additional statements needed to do so are hardly good justification for it. Hence, copy that data and then clear the context as other child processes do. Because this patch changes the memory context in which a bgworker function gains control, back-patching it would be a bit risky, so we won't fix this in back branches. The "security" complaint is pretty thin anyway for generic bgworkers; only with the introduction of parallel query is there any question of running untrusted code in a bgworker process. Discussion: <14111.1470082717@sss.pgh.pa.us> --- src/backend/postmaster/postmaster.c | 17 ++++++++++++++--- src/backend/utils/mmgr/README | 10 ++++++---- 2 files changed, 20 insertions(+), 7 deletions(-) diff --git a/src/backend/postmaster/postmaster.c b/src/backend/postmaster/postmaster.c index 1813f8ca1d..f5c8e9d812 100644 --- a/src/backend/postmaster/postmaster.c +++ b/src/backend/postmaster/postmaster.c @@ -5181,7 +5181,7 @@ CountChildren(int target) /* * StartChildProcess -- start an auxiliary process for the postmaster * - * xlop determines what kind of child will be started. All child types + * "type" determines what kind of child will be started. All child types * initially go to AuxiliaryProcessMain, which will handle common setup. * * Return value of StartChildProcess is subprocess' PID, or 0 if failed @@ -5529,9 +5529,19 @@ do_start_bgworker(RegisteredBgWorker *rw) /* Close the postmaster's sockets */ ClosePostmasterPorts(false); - /* Do NOT release postmaster's working memory context */ + /* + * Before blowing away PostmasterContext, save this bgworker's + * data where it can find it. + */ + MyBgworkerEntry = (BackgroundWorker *) + MemoryContextAlloc(TopMemoryContext, sizeof(BackgroundWorker)); + memcpy(MyBgworkerEntry, &rw->rw_worker, sizeof(BackgroundWorker)); + + /* Release postmaster's working memory context */ + MemoryContextSwitchTo(TopMemoryContext); + MemoryContextDelete(PostmasterContext); + PostmasterContext = NULL; - MyBgworkerEntry = &rw->rw_worker; StartBackgroundWorker(); break; #endif @@ -5539,6 +5549,7 @@ do_start_bgworker(RegisteredBgWorker *rw) rw->rw_pid = worker_pid; rw->rw_backend->pid = rw->rw_pid; ReportBackgroundWorkerPID(rw); + break; } } diff --git a/src/backend/utils/mmgr/README b/src/backend/utils/mmgr/README index 80a7b6a453..f97d7653de 100644 --- a/src/backend/utils/mmgr/README +++ b/src/backend/utils/mmgr/README @@ -160,10 +160,12 @@ running with CurrentMemoryContext pointing here. PostmasterContext --- this is the postmaster's normal working context. After a backend is spawned, it can delete PostmasterContext to free its copy of memory the postmaster was using that it doesn't need. -(Anything that has to be passed from postmaster to backends is passed -in TopMemoryContext. The postmaster has only TopMemoryContext, -PostmasterContext, and ErrorContext --- the remaining top-level contexts -are set up in each backend during startup.) +Note that in non-EXEC_BACKEND builds, the postmaster's copy of pg_hba.conf +and pg_ident.conf data is used directly during authentication in backend +processes; so backends can't delete PostmasterContext until that's done. +(The postmaster has only TopMemoryContext, PostmasterContext, and +ErrorContext --- the remaining top-level contexts are set up in each +backend during startup.) CacheMemoryContext --- permanent storage for relcache, catcache, and related modules. This will never be reset or deleted, either, so it's