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

use confstr(_CS_DARWIN_USER_TEMP_DIR, ...) as a TMPDIR fallback on Darwin #131505

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

madsmtm
Copy link
Contributor

@madsmtm madsmtm commented Oct 10, 2024

Rebased version of #100824, FCP has completed there. Motivation from #100824 (comment):

This is a behavioral change in an edge case on Darwin platforms (macOS, iOS, ...).

Specifically, this changes it so that iff TMPDIR is unset in the environment, then we use confstr(_CS_DARWIN_USER_TEMP_DIR, ...) to query the user temporary directory (previously we just returned "/tmp"). If this fails (probably possible in a sandboxed program), only then do we fallback to "/tmp" (as before).

The motivations here are two-fold:

  1. This is better for security, and is in line with the platform security recommendations, as it is unavailable to other users (although it is the same value as seen by all other processes run by the same user).
  2. This is a more consistent fallback for when getenv("TMPDIR") is unavailable, as $TMPDIR is usually initialized to the DARWIN_USER_TEMP_DIR.

It seems quite unlikely that anybody will break because of this, and I think it falls under the carve-out we have for platform specific behavior: https://doc.rust-lang.org/nightly/std/io/index.html#platform-specific-behavior.

Closes #99608.
Closes #100824.

@rustbot label O-apple T-libs-api

r? Dylan-DPC

@rustbot
Copy link
Collaborator

rustbot commented Oct 10, 2024

r? @Mark-Simulacrum

rustbot has assigned @Mark-Simulacrum.
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

@rustbot rustbot added O-unix Operating system: Unix-like 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. O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Oct 10, 2024
@rustbot rustbot assigned Dylan-DPC and unassigned Mark-Simulacrum Oct 10, 2024
@thomcc
Copy link
Member

thomcc commented Oct 10, 2024

Rebased version of #100824

You own for doing this, it had totally fallen off my radar.

@rust-log-analyzer

This comment has been minimized.

@madsmtm
Copy link
Contributor Author

madsmtm commented Oct 10, 2024

Ah, seems like we'll have to implement this in miri too. I'll try to whip up a PR there.
@rustbot blocked

Unless you know that such functions are generally not wanted in Miri, and that we should instead ignore the test?

@rustbot rustbot added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 10, 2024
@madsmtm
Copy link
Contributor Author

madsmtm commented Oct 10, 2024

... never mind, that's a larger endeavour, and I'm not sure it's really desired for Miri to implement confstr, so I've opted to not use it when running under Miri for now.

@rustbot ready

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-apple Operating system: Apple (macOS, iOS, tvOS, visionOS, watchOS) O-unix Operating system: Unix-like 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. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consider returning /private/tmp instead of /tmp on macOS with std::env::temp_dir()
6 participants