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

ext/pcntl: pcntl_exec() using execveat whenever possible instead. #17834

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

devnexen
Copy link
Member

Allows at least to avoid original process file descriptor to leak in the new process image.
Then with Linux 6.14, we can add another layer of security to check beforehand if the executable can be actually safely executed.

@devnexen devnexen marked this pull request as ready for review February 18, 2025 23:02
@devnexen devnexen requested a review from Girgias February 18, 2025 23:02
@Girgias
Copy link
Member

Girgias commented Feb 20, 2025

I don't really know much about Linux API, maybe @NattyNarwhal has a better idea?

Allows at least to avoid original process file descriptor to leak
in the new process image.
Then with Linux 6.14, we can add another layer of security to check
beforehand if the executable can be actually safely executed.
@Girgias Girgias removed their request for review February 21, 2025 18:40
#ifdef AT_EXECVE_CHECK
// Linux >= 6.14 allows to check if `path` is allowed
// for execution per kernel security policy (w/o actually running it)
if (execveat(fd, "", argv, envp, AT_EMPTY_PATH | AT_EXECVE_CHECK) < 0) {
Copy link
Member

Choose a reason for hiding this comment

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

If we consider that the check is successful most of the time, does this really makes much sense?

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.

4 participants