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 #100824

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions Cargo.lock
Expand Up @@ -1996,9 +1996,9 @@ checksum = "830d08ce1d1d941e6b30645f1a0eb5643013d835ce3779a5fc208261dbe10f55"

[[package]]
name = "libc"
version = "0.2.135"
version = "0.2.137"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "68783febc7782c6c5cb401fbda4de5a9898be1762314da0bb2c10ced61f18b0c"
checksum = "fc7fcc620a3bff7cdd7a365be3376c97191aeaccc2a603e600951e452615bf89"
dependencies = [
"rustc-std-workspace-core",
]
Expand Down
15 changes: 12 additions & 3 deletions library/std/src/env.rs
Expand Up @@ -600,19 +600,28 @@ pub fn home_dir() -> Option<PathBuf> {
/// may result in "insecure temporary file" security vulnerabilities. Consider
/// using a crate that securely creates temporary files or directories.
///
/// Note that the returned value may be a symbolic link, not a directory.
///
/// # Platform-specific behavior
///
/// On Unix, returns the value of the `TMPDIR` environment variable if it is
/// set, otherwise for non-Android it returns `/tmp`. On Android, since there
/// is no global temporary folder (it is usually allocated per-app), it returns
/// `/data/local/tmp`.
/// set, otherwise the value is OS-specific:
/// - On Android, there is no global temporary folder (it is usually allocated
/// per-app), it returns `/data/local/tmp`.
/// - On Darwin-based OSes (macOS, iOS, etc) it returns the directory provided
/// by `confstr(_CS_DARWIN_USER_TEMP_DIR, ...)`, as recommended by [Apple's
/// security guidelines][appledoc].
/// - On all other unix-based OSes, it returns `/tmp`.
///
/// On Windows, the behavior is equivalent to that of [`GetTempPath2`][GetTempPath2] /
/// [`GetTempPath`][GetTempPath], which this function uses internally.
///
/// Note that, this [may change in the future][changes].
///
/// [changes]: io#platform-specific-behavior
/// [GetTempPath2]: https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-gettemppath2a
/// [GetTempPath]: https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-gettemppatha
/// [appledoc]: https://developer.apple.com/library/archive/documentation/Security/Conceptual/SecureCodingGuide/Articles/RaceConditions.html#//apple_ref/doc/uid/TP40002585-SW10
///
/// ```no_run
/// use std::env;
Expand Down
76 changes: 72 additions & 4 deletions library/std/src/sys/unix/os.rs
Expand Up @@ -579,12 +579,80 @@ pub fn page_size() -> usize {
unsafe { libc::sysconf(libc::_SC_PAGESIZE) as usize }
}

// Returns the value for [`confstr(key, ...)`][posix_confstr]. Currently only
// used on Darwin, but should work on any unix (in case we need to get
// `_CS_PATH` or `_CS_V[67]_ENV` in the future).
//
// [posix_confstr]:
// https://pubs.opengroup.org/onlinepubs/9699919799/functions/confstr.html
#[cfg(any(target_os = "macos", target_os = "ios", target_os = "tvos", target_os = "watchos"))]
fn confstr(key: c_int, size_hint: Option<usize>) -> io::Result<OsString> {
let mut buf: Vec<u8> = Vec::new();
let mut bytes_needed_including_nul = size_hint
.unwrap_or_else(|| {
// Treat "None" as "do an extra call to get the length". In theory
// we could move this into the loop below, but it's hard to do given
// that it isn't 100% clear if it's legal to pass 0 for `len` when
// the buffer isn't null.
unsafe { libc::confstr(key, core::ptr::null_mut(), 0) }
})
.max(1);
// If the value returned by `confstr` is greater than the len passed into
// it, then the value was truncated, meaning we need to retry. Note that
// while `confstr` results don't seem to change for a process, it's unclear
// if this is guaranteed anywhere, so looping does seem required.
while bytes_needed_including_nul > buf.capacity() {
// We write into the spare capacity of `buf`. This lets us avoid
// changing buf's `len`, which both simplifies `reserve` computation,
// allows working with `Vec<u8>` instead of `Vec<MaybeUninit<u8>>`, and
// may avoid a copy, since the Vec knows that none of the bytes are needed
// when reallocating (well, in theory anyway).
buf.reserve(bytes_needed_including_nul);
// `confstr` returns
// - 0 in the case of errors: we break and return an error.
// - The number of bytes written, iff the provided buffer is enough to
// hold the entire value: we break and return the data in `buf`.
// - Otherwise, the number of bytes needed (including nul): we go
// through the loop again.
bytes_needed_including_nul =
unsafe { libc::confstr(key, buf.as_mut_ptr().cast::<c_char>(), buf.capacity()) };
}
// `confstr` returns 0 in the case of an error.
if bytes_needed_including_nul == 0 {
return Err(io::Error::last_os_error());
}
// Safety: `confstr(..., buf.as_mut_ptr(), buf.capacity())` returned a
// non-zero value, meaning `bytes_needed_including_nul` bytes were
// initialized.
unsafe {
buf.set_len(bytes_needed_including_nul);
// Remove the NUL-terminator.
let last_byte = buf.pop();
// ... and smoke-check that it *was* a NUL-terminator.
assert_eq!(last_byte, Some(0), "`confstr` provided a string which wasn't nul-terminated");
};
Ok(OsString::from_vec(buf))
}

#[cfg(target_vendor = "apple")]
fn darwin_temp_dir() -> PathBuf {
confstr(libc::_CS_DARWIN_USER_TEMP_DIR, Some(64)).map(PathBuf::from).unwrap_or_else(|_| {
// It failed for whatever reason (there are several possible reasons),
// so return the global one.
PathBuf::from("/tmp")
})
}

pub fn temp_dir() -> PathBuf {
crate::env::var_os("TMPDIR").map(PathBuf::from).unwrap_or_else(|| {
if cfg!(target_os = "android") {
PathBuf::from("/data/local/tmp")
} else {
PathBuf::from("/tmp")
cfg_if::cfg_if! {
if #[cfg(any(target_os = "macos", target_os = "ios", target_os = "tvos", target_os = "watchos"))] {
darwin_temp_dir()
} else if #[cfg(target_os = "android")] {
PathBuf::from("/data/local/tmp")
} else {
PathBuf::from("/tmp")
}
}
})
}
Expand Down
25 changes: 25 additions & 0 deletions library/std/src/sys/unix/os/tests.rs
Expand Up @@ -21,3 +21,28 @@ fn test_parse_glibc_version() {
assert_eq!(parsed, super::parse_glibc_version(version_str));
}
}

// Smoke check `confstr`, do it for several hint values, to ensure our resizing
// logic is correct.
#[test]
#[cfg(target_os = "macos")]
fn test_confstr() {
for key in [libc::_CS_DARWIN_USER_TEMP_DIR, libc::_CS_PATH] {
let value_nohint = super::confstr(key, None).unwrap_or_else(|e| {
panic!("confstr({key}, None) failed: {e:?}");
});
let end = (value_nohint.len() + 1) * 2;
for hint in 0..end {
assert_eq!(
super::confstr(key, Some(hint)).as_deref().ok(),
Some(&*value_nohint),
"confstr({key}, Some({hint})) failed",
);
}
}
// Smoke check that we don't loop forever or something if the input was not valid.
for hint in [None, Some(0), Some(1)] {
let hopefully_invalid = 123456789_i32;
assert!(super::confstr(hopefully_invalid, hint).is_err());
}
}