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

Commit fa31b6f

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 a311d2a commit fa31b6f

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
@@ -118,6 +118,9 @@ static volatile sig_atomic_t waiting = false;
118118
static int selfpipe_readfd = -1;
119119
static int selfpipe_writefd = -1;
120120

121+
/* Process owning the self-pipe --- needed for checking purposes */
122+
static int selfpipe_owner_pid = 0;
123+
121124
/* Private function prototypes */
122125
static void sendSelfPipeByte(void);
123126
static void drainSelfPipe(void);
@@ -146,31 +149,72 @@ InitializeLatchSupport(void)
146149
#ifndef WIN32
147150
int pipefd[2];
148151

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

151188
/*
152189
* Set up the self-pipe that allows a signal handler to wake up the
153190
* poll()/epoll_wait() in WaitLatch. Make the write-end non-blocking, so
154191
* that SetLatch won't block if the event has already been set many times
155192
* filling the kernel buffer. Make the read-end non-blocking too, so that
156193
* we can easily clear the pipe by reading until EAGAIN or EWOULDBLOCK.
194+
* Also, make both FDs close-on-exec, since we surely do not want any
195+
* child processes messing with them.
157196
*/
158197
if (pipe(pipefd) < 0)
159198
elog(FATAL, "pipe() failed: %m");
160199
if (fcntl(pipefd[0], F_SETFL, O_NONBLOCK) == -1)
161-
elog(FATAL, "fcntl() failed on read-end of self-pipe: %m");
200+
elog(FATAL, "fcntl(F_SETFL) failed on read-end of self-pipe: %m");
162201
if (fcntl(pipefd[1], F_SETFL, O_NONBLOCK) == -1)
163-
elog(FATAL, "fcntl() failed on write-end of self-pipe: %m");
202+
elog(FATAL, "fcntl(F_SETFL) failed on write-end of self-pipe: %m");
203+
if (fcntl(pipefd[0], F_SETFD, FD_CLOEXEC) == -1)
204+
elog(FATAL, "fcntl(F_SETFD) failed on read-end of self-pipe: %m");
205+
if (fcntl(pipefd[1], F_SETFD, FD_CLOEXEC) == -1)
206+
elog(FATAL, "fcntl(F_SETFD) failed on write-end of self-pipe: %m");
164207

165208
selfpipe_readfd = pipefd[0];
166209
selfpipe_writefd = pipefd[1];
210+
selfpipe_owner_pid = MyProcPid;
167211
#else
168212
/* currently, nothing to do here for Windows */
169213
#endif
170214
}
171215

172216
/*
173-
* Initialize a backend-local latch.
217+
* Initialize a process-local latch.
174218
*/
175219
void
176220
InitLatch(volatile Latch *latch)
@@ -181,7 +225,7 @@ InitLatch(volatile Latch *latch)
181225

182226
#ifndef WIN32
183227
/* Assert InitializeLatchSupport has been called in this process */
184-
Assert(selfpipe_readfd >= 0);
228+
Assert(selfpipe_readfd >= 0 && selfpipe_owner_pid == MyProcPid);
185229
#else
186230
latch->event = CreateEvent(NULL, TRUE, FALSE, NULL);
187231
if (latch->event == NULL)
@@ -199,6 +243,10 @@ InitLatch(volatile Latch *latch)
199243
* containing the latch with ShmemInitStruct. (The Unix implementation
200244
* doesn't actually require that, but the Windows one does.) Because of
201245
* this restriction, we have no concurrency issues to worry about here.
246+
*
247+
* Note that other handles created in this module are never marked as
248+
* inheritable. Thus we do not need to worry about cleaning up child
249+
* process references to postmaster-private latches or WaitEventSets.
202250
*/
203251
void
204252
InitSharedLatch(volatile Latch *latch)
@@ -244,7 +292,7 @@ OwnLatch(volatile Latch *latch)
244292

245293
#ifndef WIN32
246294
/* Assert InitializeLatchSupport has been called in this process */
247-
Assert(selfpipe_readfd >= 0);
295+
Assert(selfpipe_readfd >= 0 && selfpipe_owner_pid == MyProcPid);
248296
#endif
249297

250298
if (latch->owner_pid != 0)
@@ -277,7 +325,7 @@ DisownLatch(volatile Latch *latch)
277325
* is incurred when WL_TIMEOUT is given, so avoid using a timeout if possible.
278326
*
279327
* The latch must be owned by the current process, ie. it must be a
280-
* backend-local latch initialized with InitLatch, or a shared latch
328+
* process-local latch initialized with InitLatch, or a shared latch
281329
* associated with the current process by calling OwnLatch.
282330
*
283331
* Returns bit mask indicating which condition(s) caused the wake-up. Note
@@ -517,9 +565,9 @@ CreateWaitEventSet(MemoryContext context, int nevents)
517565
set->nevents_space = nevents;
518566

519567
#if defined(WAIT_USE_EPOLL)
520-
set->epoll_fd = epoll_create(nevents);
568+
set->epoll_fd = epoll_create1(EPOLL_CLOEXEC);
521569
if (set->epoll_fd < 0)
522-
elog(ERROR, "epoll_create failed: %m");
570+
elog(ERROR, "epoll_create1 failed: %m");
523571
#elif defined(WAIT_USE_WIN32)
524572

525573
/*
@@ -540,6 +588,12 @@ CreateWaitEventSet(MemoryContext context, int nevents)
540588

541589
/*
542590
* Free a previously created WaitEventSet.
591+
*
592+
* Note: preferably, this shouldn't have to free any resources that could be
593+
* inherited across an exec(). If it did, we'd likely leak those resources in
594+
* many scenarios. For the epoll case, we ensure that by setting FD_CLOEXEC
595+
* when the FD is created. For the Windows case, we assume that the handles
596+
* involved are non-inheritable.
543597
*/
544598
void
545599
FreeWaitEventSet(WaitEventSet *set)
@@ -586,7 +640,7 @@ FreeWaitEventSet(WaitEventSet *set)
586640
* used to modify previously added wait events using ModifyWaitEvent().
587641
*
588642
* In the WL_LATCH_SET case the latch must be owned by the current process,
589-
* i.e. it must be a backend-local latch initialized with InitLatch, or a
643+
* i.e. it must be a process-local latch initialized with InitLatch, or a
590644
* shared latch associated with the current process by calling OwnLatch.
591645
*
592646
* In the WL_SOCKET_READABLE/WRITEABLE case, EOF and error conditions are

0 commit comments

Comments
 (0)
0