Skip to content

Commit 1da569c

Browse files
committed
Don't leak descriptors into subprograms.
Open long-lived data and WAL file descriptors with O_CLOEXEC. This flag was introduced by SUSv4 (POSIX.1-2008), and by now all of our target Unix systems have it. Our open() implementation for Windows already had that behavior, so provide a dummy O_CLOEXEC flag on that platform. For now, callers of open() and the "thin" wrappers in fd.c that deal in raw descriptors need to pass in O_CLOEXEC explicitly if desired. This commit does that for WAL files, and automatically for everything accessed via VFDs including SMgrRelation and BufFile. (With more discussion we might decide to turn it on automatically for the thin open()-wrappers too to avoid risk of missing places that need it, but these are typically used for short-lived descriptors where we don't expect to fork/exec, and it's remotely possible that extensions could be using these APIs and passing descriptors to subprograms deliberately, so that hasn't been done here.) Do the same for sockets and the postmaster pipe with FD_CLOEXEC. (Later commits might use modern interfaces to remove these extra fcntl() calls and more where possible, but we'll need them as a fallback for a couple of systems, so do it that way in this initial commit.) With this change, subprograms executed for archiving, copying etc will no longer have access to the server's descriptors, other than the ones that we decide to pass down. Reviewed-by: Andres Freund <[email protected]> (earlier version) Discussion: https://postgr.es/m/CA%2BhUKGKb6FsAdQWcRL35KJsftv%2B9zXqQbzwkfRf1i0J2e57%2BhQ%40mail.gmail.com
1 parent 6b661b0 commit 1da569c

File tree

5 files changed

+36
-3
lines changed

5 files changed

+36
-3
lines changed

src/backend/access/transam/xlog.c

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2936,7 +2936,8 @@ XLogFileInitInternal(XLogSegNo logsegno, TimeLineID logtli,
29362936
* Try to use existent file (checkpoint maker may have created it already)
29372937
*/
29382938
*added = false;
2939-
fd = BasicOpenFile(path, O_RDWR | PG_BINARY | get_sync_bit(sync_method));
2939+
fd = BasicOpenFile(path, O_RDWR | PG_BINARY | O_CLOEXEC |
2940+
get_sync_bit(sync_method));
29402941
if (fd < 0)
29412942
{
29422943
if (errno != ENOENT)
@@ -3097,7 +3098,8 @@ XLogFileInit(XLogSegNo logsegno, TimeLineID logtli)
30973098
return fd;
30983099

30993100
/* Now open original target segment (might not be file I just made) */
3100-
fd = BasicOpenFile(path, O_RDWR | PG_BINARY | get_sync_bit(sync_method));
3101+
fd = BasicOpenFile(path, O_RDWR | PG_BINARY | O_CLOEXEC |
3102+
get_sync_bit(sync_method));
31013103
if (fd < 0)
31023104
ereport(ERROR,
31033105
(errcode_for_file_access(),
@@ -3328,7 +3330,8 @@ XLogFileOpen(XLogSegNo segno, TimeLineID tli)
33283330

33293331
XLogFilePath(path, tli, segno, wal_segment_size);
33303332

3331-
fd = BasicOpenFile(path, O_RDWR | PG_BINARY | get_sync_bit(sync_method));
3333+
fd = BasicOpenFile(path, O_RDWR | PG_BINARY | O_CLOEXEC |
3334+
get_sync_bit(sync_method));
33323335
if (fd < 0)
33333336
ereport(PANIC,
33343337
(errcode_for_file_access(),

src/backend/libpq/pqcomm.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -200,6 +200,13 @@ pq_init(void)
200200
(errmsg("could not set socket to nonblocking mode: %m")));
201201
#endif
202202

203+
#ifndef WIN32
204+
205+
/* Don't give the socket to any subprograms we execute. */
206+
if (fcntl(MyProcPort->sock, F_SETFD, FD_CLOEXEC) < 0)
207+
elog(FATAL, "fcntl(F_SETFD) failed on socket: %m");
208+
#endif
209+
203210
FeBeWaitSet = CreateWaitEventSet(TopMemoryContext, FeBeWaitSetNEvents);
204211
socket_pos = AddWaitEventToSet(FeBeWaitSet, WL_SOCKET_WRITEABLE,
205212
MyProcPort->sock, NULL, NULL);

src/backend/storage/file/fd.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1515,6 +1515,14 @@ PathNameOpenFilePerm(const char *fileName, int fileFlags, mode_t fileMode)
15151515
/* Close excess kernel FDs. */
15161516
ReleaseLruFiles();
15171517

1518+
/*
1519+
* Descriptors managed by VFDs are implicitly marked O_CLOEXEC. The
1520+
* client shouldn't be expected to know which kernel descriptors are
1521+
* currently open, so it wouldn't make sense for them to be inherited by
1522+
* executed subprograms.
1523+
*/
1524+
fileFlags |= O_CLOEXEC;
1525+
15181526
vfdP->fd = BasicOpenFilePerm(fileName, fileFlags, fileMode);
15191527

15201528
if (vfdP->fd < 0)

src/backend/utils/init/miscinit.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -163,6 +163,14 @@ InitPostmasterChild(void)
163163

164164
/* Request a signal if the postmaster dies, if possible. */
165165
PostmasterDeathSignalInit();
166+
167+
/* Don't give the pipe to subprograms that we execute. */
168+
#ifndef WIN32
169+
if (fcntl(postmaster_alive_fds[POSTMASTER_FD_WATCH], F_SETFD, FD_CLOEXEC) < 0)
170+
ereport(FATAL,
171+
(errcode_for_socket_access(),
172+
errmsg_internal("could not set postmaster death monitoring pipe to FD_CLOEXEC mode: %m")));
173+
#endif
166174
}
167175

168176
/*

src/include/port/win32_port.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -353,6 +353,13 @@ extern int _pglstat64(const char *name, struct stat *buf);
353353
*/
354354
#define O_DSYNC 0x0080
355355

356+
/*
357+
* Our open() replacement does not create inheritable handles, so it is safe to
358+
* ignore O_CLOEXEC. (If we were using Windows' own open(), it might be
359+
* necessary to convert this to _O_NOINHERIT.)
360+
*/
361+
#define O_CLOEXEC 0
362+
356363
/*
357364
* Supplement to <errno.h>.
358365
*

0 commit comments

Comments
 (0)