diff --git a/src/include/port.h b/src/include/port.h index 159c2bcd7e3..672b880d40b 100644 --- a/src/include/port.h +++ b/src/include/port.h @@ -359,7 +359,6 @@ extern bool rmtree(const char *path, bool rmtopdir); * open() and fopen() replacements to allow deletion of open files and * passing of other special options. */ -#define O_DIRECT 0x80000000 extern HANDLE pgwin32_open_handle(const char *, int, bool); extern int pgwin32_open(const char *, int,...); extern FILE *pgwin32_fopen(const char *, const char *); diff --git a/src/include/port/win32_port.h b/src/include/port/win32_port.h index f54ccef7db8..8194714976f 100644 --- a/src/include/port/win32_port.h +++ b/src/include/port/win32_port.h @@ -335,18 +335,15 @@ extern int _pglstat64(const char *name, struct stat *buf); /* * Supplement to . - * This is the same value as _O_NOINHERIT in the MS header file. This is - * to ensure that we don't collide with a future definition. It means - * we cannot use _O_NOINHERIT ourselves. + * + * We borrow bits from the high end when we have to, to avoid colliding with + * the system-defined values. Our open() replacement in src/port/open.c + * converts these to the equivalent CreateFile() flags, along with the ones + * from fcntl.h. */ -#define O_DSYNC 0x0080 - -/* - * Our open() replacement does not create inheritable handles, so it is safe to - * 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 _O_NOINHERIT +#define O_DIRECT 0x80000000 +#define O_DSYNC 0x04000000 /* * Supplement to . diff --git a/src/port/open.c b/src/port/open.c index 4a31c5d7b77..ec0fbf29e92 100644 --- a/src/port/open.c +++ b/src/port/open.c @@ -74,13 +74,23 @@ 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 this way avoids rare leaks in + * multi-threaded programs that create processes, just like POSIX + * O_CLOEXEC. + */ + sa.bInheritHandle = !(fileFlags & O_CLOEXEC); + while ((h = CreateFile(fileName, /* cannot use O_RDONLY, as it == 0 */ (fileFlags & O_RDWR) ? (GENERIC_WRITE | GENERIC_READ) : diff --git a/src/test/modules/Makefile b/src/test/modules/Makefile index 4a109ccbf6c..4c6d56d97d8 100644 --- a/src/test/modules/Makefile +++ b/src/test/modules/Makefile @@ -19,6 +19,7 @@ SUBDIRS = \ test_binaryheap \ test_bitmapset \ test_bloomfilter \ + test_cloexec \ test_copy_callbacks \ test_custom_rmgrs \ test_custom_stats \ diff --git a/src/test/modules/meson.build b/src/test/modules/meson.build index 2806db485d3..068fd859a8f 100644 --- a/src/test/modules/meson.build +++ b/src/test/modules/meson.build @@ -19,6 +19,7 @@ subdir('test_aio') subdir('test_binaryheap') subdir('test_bitmapset') subdir('test_bloomfilter') +subdir('test_cloexec') subdir('test_copy_callbacks') subdir('test_custom_rmgrs') subdir('test_custom_stats') diff --git a/src/test/modules/test_cloexec/Makefile b/src/test/modules/test_cloexec/Makefile new file mode 100644 index 00000000000..70d38575e26 --- /dev/null +++ b/src/test/modules/test_cloexec/Makefile @@ -0,0 +1,30 @@ +# src/test/modules/test_cloexec/Makefile + +# This module is for Windows only +ifeq ($(PORTNAME),win32) + +MODULE_big = test_cloexec +OBJS = \ + $(WIN32RES) \ + test_cloexec.o + +PGFILEDESC = "test_cloexec - test O_CLOEXEC flag handling" + +# Build as a standalone program, not a shared library +PROGRAM = test_cloexec +override CPPFLAGS := -I$(libpq_srcdir) $(CPPFLAGS) + +TAP_TESTS = 1 + +endif + +ifdef USE_PGXS +PG_CONFIG = pg_config +PGXS := $(shell $(PG_CONFIG) --pgxs) +include $(PGXS) +else +subdir = src/test/modules/test_cloexec +top_builddir = ../../../.. +include $(top_builddir)/src/Makefile.global +include $(top_srcdir)/contrib/contrib-global.mk +endif diff --git a/src/test/modules/test_cloexec/meson.build b/src/test/modules/test_cloexec/meson.build new file mode 100644 index 00000000000..63c8658b04e --- /dev/null +++ b/src/test/modules/test_cloexec/meson.build @@ -0,0 +1,26 @@ +# src/test/modules/test_cloexec/meson.build + +# This test is Windows-only +if host_system != 'windows' + subdir_done() +endif + +test_cloexec_sources = files('test_cloexec.c') + +test_cloexec = executable('test_cloexec', + test_cloexec_sources, + dependencies: [frontend_code], + link_with: [pgport[''], pgcommon['']], + kwargs: default_bin_args, +) + +tests += { + 'name': 'test_cloexec', + 'sd': meson.current_source_dir(), + 'bd': meson.current_build_dir(), + 'tap': { + 'tests': [ + 't/001_cloexec.pl', + ], + }, +} diff --git a/src/test/modules/test_cloexec/t/001_cloexec.pl b/src/test/modules/test_cloexec/t/001_cloexec.pl new file mode 100644 index 00000000000..4c56eb3a4be --- /dev/null +++ b/src/test/modules/test_cloexec/t/001_cloexec.pl @@ -0,0 +1,60 @@ +# Test O_CLOEXEC flag handling on Windows +# +# This test verifies that file handles opened with O_CLOEXEC are not +# inherited by child processes, while handles opened without O_CLOEXEC +# are inherited. + +use strict; +use warnings FATAL => 'all'; +use PostgreSQL::Test::Utils; +use Test::More; +use IPC::Run qw(run); +use File::Spec; +use Cwd 'abs_path'; + +if (!$PostgreSQL::Test::Utils::windows_os) +{ + plan skip_all => 'test is Windows-specific'; +} + +plan tests => 1; + +my $test_prog; +foreach my $dir (split(/$Config::Config{path_sep}/, $ENV{PATH})) +{ + my $candidate = File::Spec->catfile($dir, 'test_cloexec.exe'); + if (-f $candidate && -x $candidate) + { + $test_prog = $candidate; + last; + } +} + +if (!$test_prog) +{ + $test_prog = './test_cloexec.exe'; +} + +if (!-f $test_prog) +{ + BAIL_OUT("test program not found: $test_prog"); +} + +note("Using test program: $test_prog"); + +my ($stdout, $stderr); +my $result = run [ $test_prog ], '>', \$stdout, '2>', \$stderr; + +note("Test program output:"); +note($stdout) if $stdout; + +if ($stderr) +{ + diag("Test program stderr:"); + diag($stderr); +} + +ok($result && $stdout =~ /SUCCESS.*O_CLOEXEC behavior verified/s, + "O_CLOEXEC prevents handle inheritance"); + +done_testing(); \ No newline at end of file diff --git a/src/test/modules/test_cloexec/test_cloexec.c b/src/test/modules/test_cloexec/test_cloexec.c new file mode 100644 index 00000000000..9f645451684 --- /dev/null +++ b/src/test/modules/test_cloexec/test_cloexec.c @@ -0,0 +1,262 @@ +/*------------------------------------------------------------------------- + * + * test_cloexec.c + * Test O_CLOEXEC flag handling on Windows + * + * This program tests that: + * 1. File handles opened with O_CLOEXEC are NOT inherited by child processes + * 2. File handles opened without O_CLOEXEC ARE inherited by child processes + * + * Portions Copyright (c) 1996-2025, PostgreSQL Global Development Group + * + *------------------------------------------------------------------------- + */ + +#include "postgres.h" + +#include +#include + +#ifdef WIN32 +#include +#endif + +#include "common/file_utils.h" +#include "port.h" + +static void run_parent_tests(const char *testfile1, const char *testfile2); +static void run_child_tests(const char *handle1_str, const char *handle2_str); +static bool try_write_to_handle(HANDLE h, const char *label); + +int +main(int argc, char *argv[]) +{ + char testfile1[MAXPGPATH]; + char testfile2[MAXPGPATH]; + + /* Windows-only test */ +#ifndef WIN32 + fprintf(stderr, "This test only runs on Windows\n"); + return 0; +#endif + + if (argc == 3) + { + /* + * Child mode: receives two handle values as hex strings and attempts + * to write to them. + */ + run_child_tests(argv[1], argv[2]); + return 0; + } + else if (argc == 1) + { + /* Parent mode: opens files and spawns child */ + snprintf(testfile1, sizeof(testfile1), "test_cloexec_1_%d.tmp", (int) getpid()); + snprintf(testfile2, sizeof(testfile2), "test_cloexec_2_%d.tmp", (int) getpid()); + + run_parent_tests(testfile1, testfile2); + + /* Clean up test files */ + unlink(testfile1); + unlink(testfile2); + + return 0; + } + else + { + fprintf(stderr, "Usage: %s [handle1_hex handle2_hex]\n", argv[0]); + return 1; + } +} + +static void +run_parent_tests(const char *testfile1, const char *testfile2) +{ +#ifdef WIN32 + int fd1, + fd2; + HANDLE h1, + h2; + char cmdline[1024]; + STARTUPINFO si; + PROCESS_INFORMATION pi; + DWORD exit_code; + + printf("Parent: Opening test files...\n"); + + /* + * Open first file WITH O_CLOEXEC - should NOT be inherited + */ + fd1 = open(testfile1, O_RDWR | O_CREAT | O_TRUNC | O_CLOEXEC, 0600); + if (fd1 < 0) + { + fprintf(stderr, "Failed to open %s: %s\n", testfile1, strerror(errno)); + exit(1); + } + + /* + * Open second file WITHOUT O_CLOEXEC - should be inherited + */ + fd2 = open(testfile2, O_RDWR | O_CREAT | O_TRUNC, 0600); + if (fd2 < 0) + { + fprintf(stderr, "Failed to open %s: %s\n", testfile2, strerror(errno)); + close(fd1); + exit(1); + } + + /* Get Windows HANDLEs from file descriptors */ + h1 = (HANDLE) _get_osfhandle(fd1); + h2 = (HANDLE) _get_osfhandle(fd2); + + if (h1 == INVALID_HANDLE_VALUE || h2 == INVALID_HANDLE_VALUE) + { + fprintf(stderr, "Failed to get OS handles\n"); + close(fd1); + close(fd2); + exit(1); + } + + printf("Parent: fd1=%d (O_CLOEXEC) -> HANDLE=%p\n", fd1, h1); + printf("Parent: fd2=%d (no O_CLOEXEC) -> HANDLE=%p\n", fd2, h2); + + /* + * Spawn child process with bInheritHandles=TRUE, passing handle values as + * hex strings + */ + snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p", + GetCommandLine(), h1, h2); + + /* + * Find the actual executable path by removing any arguments from + * GetCommandLine(). + */ + { + char exe_path[MAX_PATH]; + char *space_pos; + + GetModuleFileName(NULL, exe_path, sizeof(exe_path)); + snprintf(cmdline, sizeof(cmdline), "\"%s\" %p %p", + exe_path, h1, h2); + } + + memset(&si, 0, sizeof(si)); + si.cb = sizeof(si); + memset(&pi, 0, sizeof(pi)); + + printf("Parent: Spawning child process...\n"); + printf("Parent: Command line: %s\n", cmdline); + + if (!CreateProcess(NULL, /* application name */ + cmdline, /* command line */ + NULL, /* process security attributes */ + NULL, /* thread security attributes */ + TRUE, /* bInheritHandles - CRITICAL! */ + 0, /* creation flags */ + NULL, /* environment */ + NULL, /* current directory */ + &si, /* startup info */ + &pi)) /* process information */ + { + fprintf(stderr, "CreateProcess failed: %lu\n", GetLastError()); + close(fd1); + close(fd2); + exit(1); + } + + printf("Parent: Waiting for child process...\n"); + + /* Wait for child to complete */ + WaitForSingleObject(pi.hProcess, INFINITE); + GetExitCodeProcess(pi.hProcess, &exit_code); + + CloseHandle(pi.hProcess); + CloseHandle(pi.hThread); + + close(fd1); + close(fd2); + + printf("Parent: Child exit code: %lu\n", exit_code); + + if (exit_code == 0) + printf("Parent: SUCCESS - O_CLOEXEC behavior verified\n"); + else + { + printf("Parent: FAILURE - O_CLOEXEC not working correctly\n"); + exit(1); + } +#endif +} + +static void +run_child_tests(const char *handle1_str, const char *handle2_str) +{ +#ifdef WIN32 + HANDLE h1, + h2; + bool h1_worked, + h2_worked; + + /* Parse handle values from hex strings */ + if (sscanf(handle1_str, "%p", &h1) != 1 || + sscanf(handle2_str, "%p", &h2) != 1) + { + fprintf(stderr, "Child: Failed to parse handle values\n"); + exit(1); + } + + printf("Child: Received HANDLE1=%p (should fail - O_CLOEXEC)\n", h1); + printf("Child: Received HANDLE2=%p (should work - no O_CLOEXEC)\n", h2); + + /* Try to write to both handles */ + h1_worked = try_write_to_handle(h1, "HANDLE1"); + h2_worked = try_write_to_handle(h2, "HANDLE2"); + + printf("Child: HANDLE1 (O_CLOEXEC): %s\n", + h1_worked ? "ACCESSIBLE (BAD!)" : "NOT ACCESSIBLE (GOOD!)"); + printf("Child: HANDLE2 (no O_CLOEXEC): %s\n", + h2_worked ? "ACCESSIBLE (GOOD!)" : "NOT ACCESSIBLE (BAD!)"); + + /* + * For O_CLOEXEC to work correctly, h1 should NOT be accessible (h1_worked + * == false) and h2 SHOULD be accessible (h2_worked == true). + */ + if (!h1_worked && h2_worked) + { + printf("Child: Test PASSED - O_CLOEXEC working correctly\n"); + exit(0); + } + else + { + printf("Child: Test FAILED - O_CLOEXEC not working correctly\n"); + exit(1); + } +#endif +} + +static bool +try_write_to_handle(HANDLE h, const char *label) +{ +#ifdef WIN32 + const char *test_data = "test\n"; + DWORD bytes_written; + BOOL result; + + result = WriteFile(h, test_data, strlen(test_data), &bytes_written, NULL); + + if (result && bytes_written == strlen(test_data)) + { + printf("Child: Successfully wrote to %s\n", label); + return true; + } + else + { + printf("Child: Failed to write to %s (error %lu)\n", + label, GetLastError()); + return false; + } +#else + return false; +#endif +}