Prevent mutating the global environment pointer in `CommandExt::exec` and opt to use execve and resolve path manually by asder8215 · Pull Request #157144 · rust-lang/rust

asder8215 · 2026-05-30T07:04:28Z

This PR fixes the bug described in #156951 where CommandExt::exec mutating the global environment pointer could cause correctness issue with concurrent exec calls (also this function holds an environment read lock, so it shouldn't cause any writes to the global environment pointer anyways). We do everything within CommandExt::exec via execve than execvp, which means we have to resolve the path manually.

I took reference to what was done in an archived (Feb 2026) upstream mirror of glibc's execvp and execvpe. I also took reference to #55359 on how they implemented the concerned portion of CommandExt::exec with execve. There's also this OpenBSD libc implementation of execvpe that I saw, but I didn't deeply take a look at this one and saw how it differ.

Other than that, I'm unsure how to handle the error ETIMEOUT/TimedOut as glibc breaks parsing through the PATH environment variable value and returns the error while #55359 continues parsing.

Let me know if you also want me to put some of execve code in a helper function or refactor it in other ways.

rustbot · 2026-05-30T07:04:33Z

r? @joboet

rustbot has assigned @joboet.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

Owners of files modified in this PR: @ChrisDenton, libs
@ChrisDenton, libs expanded to 8 candidates
Random selection from Mark-Simulacrum, joboet

joboet

I highly recommend having a look at the POSIX specification – GNU/Linux mostly follows that, but the specification sometimes leaves some things unspecified and we should be careful not to break those.

Also, this is currently unsound, we cannot perform any memory allocation in do_exec, since it's called after fork for the standard spawn path. You probably need to preallocate some memory when creating the Command (or when setting certain parameters).

View changes since this review

joboet · 2026-05-30T12:17:43Z

+                    None => c"/bin:/usr/bin".to_bytes_with_nul(),
+                }
+            }
+            None => parent_path.as_bytes_with_nul(),


I recommend moving the environment variable lookup here so that it's only performed when actually needed.

rustbot · 2026-05-30T12:50:15Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

joboet · 2026-06-08T13:23:33Z

+            assert_failed(index, len);
+        }
+
+        self.ptrs.insert(index, item.into_raw());


This leaks the string if insert panics (see #155748, I made this mistake before).

View changes since the review

joboet · 2026-06-17T10:17:12Z

Hey T-libs,

This PR makes std responsible for the PATH-lookup of programs not spawned through posix_spawnp (which doesn't apply in cases like if the PATH of the child has changed). Unfortunately, the POSIX standard leaves quite a lot up to the implementation, meaning this could change behaviour on some platforms. I'll need to do some more research to figure out the exact list, but wanted to check with you first whether

You are happy with pursuing this further (CommandExt::exec writes to the global environment pointer while holding only a read lock #156951 cannot be fixed any other way unfortunately).
You think we should use the same lookup logic for the posix_spawn happy path too, to keep behaviour consistent.

@rustbot label +I-libs-nominated

joshtriplett · 2026-07-01T16:28:55Z

Sounds reasonable to me.

Amanieu · 2026-07-01T16:29:55Z

Based on the discussion in the meeting, I'm going to propose FCP on the following:

We are happy to move forward with this since it seems to be the only proper solution to the problem.
We don't want to change path lookup on posix_spawn. If we discover inconsistency in behavior, we should fix the exec-side path lookup.

@rfcbot merge libs

rust-rfcbot · 2026-07-01T16:29:57Z

@Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

…nd opt to use execve and resolve path manually

…confstr_as_cstring added to get a CString value returned by confstr, added CStringArray::from_ptr, added default_path and shell_argv fields to unix Command, and refactored do_exec code

…om a single passthrough in envp, match on errno values directly instead of ErrorKind, fix shell_argv CStringArray value.

…remove confstr_as_cstr, remove unnecessary shell_argv field/funtions + unused functions from CStringArray, use slice::split to delimit on colons

…cumentation for returning an error from execve

rustbot · 2026-07-07T22:06:05Z

This PR was rebased onto a different main commit. Here's a range-diff highlighting what actually changed.

Rebasing is a normal part of keeping PRs up to date, so no action is needed—this note is just to help reviewers.

…7 of confstr

asder8215 · 2026-07-07T22:12:52Z

Code's pretty much the same, documentation for the posix_confstr link refers to Issue 8 of confstr than Issue 7.

rustbot assigned joboet May 30, 2026

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels May 30, 2026

This comment has been minimized.

Sign in to view

joboet requested changes May 30, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels May 30, 2026

rustbot added the O-unix Operating system: Unix-like label May 31, 2026

asder8215 requested a review from joboet May 31, 2026 05:02

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels May 31, 2026

asder8215 commented May 31, 2026

View reviewed changes

Comment thread library/std/src/sys/process/unix/unix.rs Outdated

joboet requested changes Jun 6, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 6, 2026

asder8215 requested a review from joboet June 8, 2026 08:36

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 8, 2026

asder8215 commented Jun 8, 2026

View reviewed changes

Comment thread library/std/src/sys/process/unix/common/cstring_array.rs Outdated

This comment has been minimized.

Sign in to view

asder8215 force-pushed the commandext_exec_bug_fix branch from 213bb02 to 8319b09 Compare June 8, 2026 09:23

This comment has been minimized.

Sign in to view

asder8215 commented Jun 8, 2026

View reviewed changes

Comment thread library/std/src/sys/process/unix/unix.rs Outdated

joboet requested changes Jun 8, 2026

View reviewed changes

rustbot added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 8, 2026

joboet reviewed Jun 8, 2026

View reviewed changes

Comment thread library/std/src/sys/process/unix/unix.rs Outdated

asder8215 requested a review from joboet June 9, 2026 22:31

rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 11, 2026

joboet reviewed Jun 11, 2026

View reviewed changes

Comment thread library/std/src/sys/process/unix/unix.rs Outdated

joboet reviewed Jun 11, 2026

View reviewed changes

Comment thread library/std/src/sys/process/unix/unix.rs Outdated

asder8215 force-pushed the commandext_exec_bug_fix branch from df73301 to 0ecb419 Compare June 11, 2026 22:36

asder8215 requested a review from joboet June 11, 2026 22:40

joboet reviewed Jun 12, 2026

View reviewed changes

Comment thread library/std/src/sys/process/unix/unix.rs Outdated

Comment thread library/std/src/sys/process/unix/unix.rs

asder8215 requested a review from joboet June 12, 2026 12:38

joboet reviewed Jun 12, 2026

View reviewed changes

Comment thread library/std/src/sys/process/unix/unix.rs Outdated

asder8215 force-pushed the commandext_exec_bug_fix branch from e3d0663 to 9851b0a Compare June 12, 2026 16:19

asder8215 requested a review from joboet June 12, 2026 16:19

asder8215 force-pushed the commandext_exec_bug_fix branch 2 times, most recently from b4723b8 to df00000 Compare June 12, 2026 16:21

rustbot added the I-libs-nominated Nominated for discussion during a libs team meeting. label Jun 17, 2026

rust-rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Jul 1, 2026

This comment has been minimized.

Sign in to view

asder8215 added 5 commits July 7, 2026 17:57

Prevent mutating the global environment pointer in CommandExt::exec a…

ea13793

…nd opt to use execve and resolve path manually

Perform all memory allocations necessary for do_exec before forking; …

397f128

…confstr_as_cstring added to get a CString value returned by confstr, added CStringArray::from_ptr, added default_path and shell_argv fields to unix Command, and refactored do_exec code

Pre-allocate a buffer with the maximum path length that we'll have fr…

ca3dc68

…om a single passthrough in envp, match on errno values directly instead of ErrorKind, fix shell_argv CStringArray value.

Search for PATH environment variable from maybe_envp/environ() once, …

29d59f4

…remove confstr_as_cstr, remove unnecessary shell_argv field/funtions + unused functions from CStringArray, use slice::split to delimit on colons

Account for paths with separator byte at the end and added further do…

4ca0e7b

…cumentation for returning an error from execve

asder8215 force-pushed the commandext_exec_bug_fix branch from df00000 to 4ca0e7b Compare July 7, 2026 22:06

Update documentation on confstr to refer to Issue 8 instead of Issue …

a288cc9

…7 of confstr

nia-e removed the I-libs-nominated Nominated for discussion during a libs team meeting. label Jul 8, 2026

Uh oh!

Uh oh!

Conversation

asder8215 commented May 30, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented May 30, 2026

Uh oh!

This comment has been minimized.

joboet left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joboet May 30, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rustbot commented May 30, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

Uh oh!

Uh oh!

Uh oh!

joboet Jun 8, 2026 • edited by rustbot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

joboet commented Jun 17, 2026

Uh oh!

joshtriplett commented Jul 1, 2026

Uh oh!

Amanieu commented Jul 1, 2026

Uh oh!

rust-rfcbot commented Jul 1, 2026 • edited by cuviper Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

rustbot commented Jul 7, 2026

Uh oh!

asder8215 commented Jul 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

asder8215 commented May 30, 2026 •

edited by rustbot

Loading

joboet left a comment •

edited

Loading

joboet Jun 8, 2026 •

edited by rustbot

Loading

rust-rfcbot commented Jul 1, 2026 •

edited by cuviper

Loading