Skip to content

ext/session: Open session files with O_CLOEXEC - #24376

Merged
jorgsowa merged 2 commits into
php:masterfrom
jorgsowa:session-files-fewer-syscalls
Oct 16, 2026
Merged

jorgsowa merged 2 commits into
php:masterfrom
jorgsowa:session-files-fewer-syscalls

Conversation

@jorgsowa

Copy link
Copy Markdown
Contributor

Set close-on-exec atomically when opening the session file instead of issuing a separate fcntl(F_SETFD) call afterwards. This saves one syscall per request and closes the window in which the descriptor could leak into a child process forked by another thread. The fcntl() fallback is kept for platforms without O_CLOEXEC.

@jorgsowa jorgsowa self-assigned this Oct 16, 2026
@jorgsowa
jorgsowa requested a review from Girgias as a code owner October 16, 2026 18:15
@Girgias
Girgias requested a review from devnexen October 16, 2026 18:17
Comment thread ext/session/mod_files.c Outdated

/* O_NOFOLLOW to prevent us from following evil symlinks */
data->fd = VCWD_OPEN_MODE(buf, O_CREAT | O_RDWR | O_BINARY | O_NOFOLLOW, data->filemode);
data->fd = VCWD_OPEN_MODE(buf, O_CREAT | O_RDWR | O_BINARY | O_NOFOLLOW

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if I were you, I would build the flag via a variable before the call instead wdyt ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense and fixes Windows tests, thanks.

Set close-on-exec atomically when opening the session file instead of
issuing a separate fcntl(F_SETFD) call afterwards. This saves one
syscall per request and closes the window in which the descriptor could
leak into a child process forked by another thread. The fcntl() fallback
is kept for platforms without O_CLOEXEC.
@jorgsowa
jorgsowa force-pushed the session-files-fewer-syscalls branch from f03b954 to d85d356 Compare October 16, 2026 19:02
@jorgsowa
jorgsowa requested a review from devnexen October 16, 2026 19:17
@jorgsowa
jorgsowa merged commit 6e4082b into php:master Oct 16, 2026
16 of 18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants