8000 Make latch.c more paranoid about child-process cases. · shelljke/postgres@866452c · GitHub
[go: up one dir, main page]
Skip to content

Commit 866452c

Browse files
tglsfdctglsfdc
committed
Make latch.c more paranoid about child-process cases.
Although the postmaster doesn't currently create a self-pipe or any latches, there's discussion of it doing so in future. It's also conceivable that a shared_preload_libraries extension would try to create such a thing in the postmaster process today. In that case the self-pipe FDs would be inherited by forked child processes. latch.c was entirely unprepared for such a case and could suffer an assertion failure, or worse try to use the inherited pipe if somebody called WaitLatch without having called InitializeLatchSupport in that process. Make it keep track of whether InitializeLatchSupport has been called in the *current* process, and do the right thing if state has been inherited from a parent. Apply FD_CLOEXEC to file descriptors created in latch.c (the self-pipe, as well as epoll event sets). This ensures that child processes spawned in backends, the archiver, etc cannot accidentally or intentionally mess with these FDs. It also ensures that we end up with the right state for the self-pipe in EXEC_BACKEND processes, which otherwise wouldn't know to close the postmaster's self-pipe FDs. Back-patch to 9.6, mainly to keep latch.c looking similar in all branches it exists in. Discussion: https://postgr.es/m/8322.1493240739@sss.pgh.pa.us
1 parent e880df2 commit 866452c

File tree

1 file changed

+64
-10
lines changed

1 file changed

+64
-10
lines changed

‎src/backend/storage/ipc/latch.c

Lines changed: 64 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,9 @@ static volatile sig_atomic_t waiting = false;
130130
static int selfpipe_readfd = -1;
131131
static int selfpipe_writefd = -1;
132132

133+
/* Process owning the self-pipe --- needed for checking purposes */
134+
static int selfpipe_owner_pid = 0;
135+
133136
/* Private function prototypes */
134137
static void sendSelfPipeByte(void);
135138
static void drainSelfPipe(void);
@@ -158,31 +161,72 @@ InitializeLatchSupport(void)
158161
#ifndef WIN32
159162
int pipefd[2];
160163

161-
Assert(selfpipe_readfd == -1);
164< 10000 /code>+
if (IsUnderPostmaster)
165+
{
166+
/*
167+
* We might have inherited connections to a self-pipe created by the
168+
* postmaster. It's critical that child processes create their own
169+
* self-pipes, of course, and we really want them to close the
170+
* inherited FDs for safety's sake.
171+
*/
172+
if (selfpipe_owner_pid != 0)
173+
{
174+
/* Assert we go through here but once in a child process */
175+
Assert(selfpipe_owner_pid != MyProcPid);
176+
/* Release postmaster's pipe FDs; ignore any error */
177+
(void) close(selfpipe_readfd);
178+
(void) close(selfpipe_writefd);
179+
/* Clean up, just for safety's sake; we'll set these below */
180+
selfpipe_readfd = selfpipe_writefd = -1;
181+
selfpipe_owner_pid = 0;
182+
}
183+
else
184+
{
185+
/*
186+
* Postmaster didn't create a self-pipe ... or else we're in an
187+
* EXEC_BACKEND build, in which case it doesn't matter since the
188+
* postmaster's pipe FDs were closed by the action of FD_CLOEXEC.
189+
*/
190+
Assert(selfpipe_readfd == -1);
191+
}
192+
}
193+
else
194+
{
195+
/* In postmaster or standalone backend, assert we do this but once */
196+
Assert(selfpipe_readfd == -1);
197+
Assert(selfpipe_owner_pid == 0);
198+
}
162199

163200
/*
164201
* Set up the self-pipe that allows a signal handler to wake up the
165202
* select() in WaitLatch. Make the write-end non-blocking, so that
166203
* SetLatch won't block if the event has already been set many times
167204
* filling the kernel buffer. Make the read-end non-blocking too, so that
168205
* we can easily clear the pipe by reading until EAGAIN or EWOULDBLOCK.
206+
* Also, make both FDs close-on-exec, since we surely do not want any
207+
* child processes messing with them.
169208
*/
170209
if (pipe(pipefd) < 0)
171210
elog(FATAL, "pipe() failed: %m");
172211
if (fcntl(pipefd[0], F_SETFL, O_NONBLOCK) == -1)
173-
elog(FATAL, "fcntl() failed on read-end of self-pipe: %m");
212+
elog(FATAL, "fcntl(F_SETFL) failed on read-end of self-pipe: %m");
174213
if (fcntl(pipefd[1], F_SETFL, O_NONBLOCK) == -1)
175-
elog(FATAL, "fcntl() failed on write-end of self-pipe: %m");
214+
elog(FATAL, "fcntl(F_SETFL) failed on write-end of self-pipe: %m");
215+
if (fcntl(pipefd[0], F_SETFD, FD_CLOEXEC) == -1)
216+
elog(FATAL, "fcntl(F_SETFD) failed on read-end of self-pipe: %m");
217+
if (fcntl(pipefd[1], F_SETFD, FD_CLOEXEC) == -1)
218+
elog(FATAL, "fcntl(F_SETFD) failed on write-end of self-pipe: %m");
176219

177220
selfpipe_readfd = pipefd[0];
178221
selfpipe_writefd = pipefd[1];
222+
selfpipe_owner_pid = MyProcPid;
179223
#else
180224
/* currently, nothing to do here for Windows */
181225
#endif
182226
}
183227

184228
/*
185-
* Initialize a backend-local latch.
229+
* Initialize a process-local latch.
186230
*/
187231
void
188232
InitLatch(volatile Latch *latch)
@@ -193,7 +237,7 @@ InitLatch(volatile Latch *latch)
193237

194238
#ifndef WIN32
195239
/* Assert InitializeLatchSupport has been called in this process */
196-
Assert(selfpipe_readfd >= 0);
240+
Assert(selfpipe_readfd >= 0 && selfpipe_owner_pid == MyProcPid);
197241
#else
198242
latch->event = CreateEvent(NULL, TRUE, FALSE, NULL);
199243
if (latch->event == NULL)
@@ -211,6 +255,10 @@ InitLatch(volatile Latch *latch)
211255
* containing the latch with ShmemInitStruct. (The Unix implementation
212256
* doesn't actually require that, but the Windows one does.) Because of
213257
* this restriction, we have no concurrency issues to worry about here.
258+
*
259+
* Note that other handles created in this module are never marked as
260+
* inheritable. Thus we do not need to worry about cleaning up child
261+
* process references to postmaster-private latches or WaitEventSets.
214262
*/
215263
void
216264
InitSharedLatch(volatile Latch *latch)
@@ -256,7 +304,7 @@ OwnLatch(volatile Latch *latch)
256304

257305
#ifndef WIN32
258306
/* Assert InitializeLatchSupport has been called in this process */
259-
Assert(selfpipe_readfd >= 0);
307+
Assert(selfpipe_readfd >= 0 && selfpipe_owner_pid == MyProcPid);
260308
#endif
261309

262310
if (latch->owner_pid != 0)
@@ -289,7 +337,7 @@ DisownLatch(volatile Latch *latch)
289337
* is incurred when WL_TIMEOUT is given, so avoid using a timeout if possible.
290338
*
291339
* The latch must be owned by the current process, ie. it must be a
292-
* backend-local latch initialized with InitLatch, or a shared latch
340+
* process-local latch initialized with InitLatch, or a shared latch
293341
* associated with the current process by calling OwnLatch.
294342
*
295343
* Returns bit mask indicating which condition(s) caused the wake-up. Note
@@ -526,9 +574,9 @@ CreateWaitEventSet(MemoryContext context, int nevents)
526574
set->nevents_space = nevents;
527575

528576
#if defined(WAIT_USE_EPOLL)
529-
set->epoll_fd = epoll_create(nevents);
577+
set->epoll_fd = epoll_create1(EPOLL_CLOEXEC);
530578
if (set->epoll_fd < 0)
531-
elog(ERROR, "epoll_create failed: %m");
579+
elog(ERROR, "epoll_create1 failed: %m");
532580
#elif defined(WAIT_USE_WIN32)
533581

534582
/*
@@ -549,6 +597,12 @@ CreateWaitEventSet(MemoryContext context, int nevents)
549597

550598
/*
551599
* Free a previously created WaitEventSet.
600+
*
601+
* Note: preferably, this shouldn't have to free any resources that could be
602+
* inherited across an exec(). If it did, we'd likely leak those resources in
603+
* many scenarios. For the epoll case, we ensure that by setting FD_CLOEXEC
604+
* when the FD is created. For the Windows case, we assume that the handles
605+
* involved are non-inheritable.
552606
*/
553607
void
554608
FreeWaitEventSet(WaitEventSet *set)
@@ -595,7 +649,7 @@ FreeWaitEventSet(WaitEventSet *set)
595649
* used to modify previously added wait events using ModifyWaitEvent().
596650
*
597651
* In the WL_LATCH_SET case the latch must be owned by the current process,
598-
* i.e. it must be a backend-local latch initialized with InitLatch, or a
652+
* i.e. it must be a process-local latch initialized with InitLatch, or a
599653
* shared latch associated with the current process by calling OwnLatch.
600654
*
601655
* In the WL_SOCKET_READABLE/WRITEABLE case, EOF and error conditions are

0 commit comments

Comments
 (0)
0