Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Protect fork/exec on targets that don't support atomic CLOEXEC #14674

Merged

Conversation

ysbaddaden
Copy link
Contributor

@ysbaddaden ysbaddaden commented Jun 7, 2024

In addition to the standard O_CLOEXEC flag to open (POSIX.1-2008), all modern POSIX systems implement non-standard syscalls (accept4, dup3 and pipe2) along with the SOCK_CLOEXEC flag, that atomically create file descriptors with the FD_CLOEXEC flag.

A notable exception is Darwin that only implements O_CLOEXEC.

We thus have to support falling back to accept, dup2 and pipe that won't set FD_CLOEXEC or SOCK_CLOEXEC atomically, which creates a time window during which another thread may fork the process before FD_CLOEXEC is set, which will leak the file descriptor to a child process.

This patch introduces a RWLock to prevent fork/exec in such situations.

Follow up of #14672, #14673 and #14675.

Prior art: Go does exactly that.

@ysbaddaden ysbaddaden changed the title RWLock to protect fork for targets that don't support all the atomic CLOEXEC syscalls Protect fork/exec on targets that don't support atomic CLOEXEC Jun 10, 2024
@ysbaddaden ysbaddaden self-assigned this Jun 13, 2024
@ysbaddaden ysbaddaden marked this pull request as ready for review June 17, 2024 10:54
@straight-shoota straight-shoota added this to the 1.13.0 milestone Jun 18, 2024
@straight-shoota straight-shoota merged commit e0754ca into crystal-lang:master Jun 20, 2024
61 checks passed
@ysbaddaden ysbaddaden deleted the fix/add-rwlock-to-fork branch June 20, 2024 08:02
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.

3 participants