From 095dd7af43c81f55270db71798ee2f0248f1086e Mon Sep 17 00:00:00 2001 From: Bryan Green Date: Fri, 31 Oct 2025 10:11:30 -0600 Subject: [PATCH] Fix O_CLOEXEC flag handling in Windows port. PostgreSQL's src/port/open.c has always set bInheritHandle = TRUE when opening files on Windows, making all file descriptors inheritable by child processes. This meant the O_CLOEXEC flag, added to many call sites by commit 1da569ca1f, was silently ignored. The original commit included a comment suggesting that our open() replacement doesn't create inheritable handles, but it appears this may have been based on a misunderstanding of the code path. In practice, the code was creating inheritable handles in all cases. This hasn't caused widespread problems because most child processes (archive_command, COPY PROGRAM, etc.) operate on file paths passed as arguments rather than inherited file descriptors. Even if a child wanted to use an inherited handle, it would need to learn the numeric handle value, which isn't passed through our IPC mechanisms. Nonetheless, the current behavior is wrong. It violates documented O_CLOEXEC semantics, contradicts our own code comments, and makes PostgreSQL behave differently on Windows than on Unix. It also creates potential issues with future code or security auditing tools. To fix, define O_CLOEXEC to a nonzero value (0x80000, in the private use range for open() flags). Then honor it in pgwin32_open_handle() by setting bInheritHandle according to whether O_CLOEXEC is specified. We set bInheritHandle directly in SECURITY_ATTRIBUTES rather than using SetHandleInformation() afterwards, as the former is simpler. Back-patch to v16 where commit 1da569ca1f introduced widespread use of O_CLOEXEC. While the practical risk is low, having code work as documented and consistently across platforms is worth the minimal risk. Bug found while reviewing an unrelated patch to pg_ctl's process spawning logic. Author: Bryan Green --- src/include/port/win32_port.h | 2 +- src/port/open.c | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h index ff7028bdc8..07fece6c46 100644 --- a/src/include/port/win32_port.h +++ b/src/include/port/win32_port.h @@ -346,7 +346,7 @@ extern int _pglstat64(const char *name, struct stat *buf); * ignore O_CLOEXEC. (If we were using Windows' own open(), it might be * necessary to convert this to _O_NOINHERIT.) */ -#define O_CLOEXEC 0 +#define O_CLOEXEC 0x80000 /* * Supplement to . diff --git a/src/port/open.c b/src/port/open.c index 4a31c5d7b7..71344b9917 100644 --- a/src/port/open.c +++ b/src/port/open.c @@ -74,13 +74,22 @@ pgwin32_open_handle(const char *fileName, int fileFlags, bool backup_semantics) /* Check that we can handle the request */ assert((fileFlags & ((O_RDONLY | O_WRONLY | O_RDWR) | O_APPEND | (O_RANDOM | O_SEQUENTIAL | O_TEMPORARY) | - _O_SHORT_LIVED | O_DSYNC | O_DIRECT | + _O_SHORT_LIVED | O_DSYNC | O_DIRECT | O_CLOEXEC | (O_CREAT | O_TRUNC | O_EXCL) | (O_TEXT | O_BINARY))) == fileFlags); sa.nLength = sizeof(sa); - sa.bInheritHandle = TRUE; sa.lpSecurityDescriptor = NULL; + /* + * If O_CLOEXEC is specified, create a non-inheritable handle. Otherwise, + * create an inheritable handle (the default Windows behavior). + * + * Note: We could instead use SetHandleInformation() after CreateFile() to + * clear HANDLE_FLAG_INHERIT, but setting bInheritHandle=FALSE is simpler + * and achieves the same result. + */ + sa.bInheritHandle = (fileFlags & O_CLOEXEC) ? FALSE : TRUE; + while ((h = CreateFile(fileName, /* cannot use O_RDONLY, as it == 0 */ (fileFlags & O_RDWR) ? (GENERIC_WRITE | GENERIC_READ) : -- 2.46.0.windows.1