Rollup merge of #114968 - ShE3py:unix-getsetenv-ub, r=thomcc

Fix UB in `std::sys::os::getenv()`

Fixes #114949.

Reduced the loops to 1k iterations (100k was taking way too long), Miri no longer shows any UB.

`@rustbot` label +A-process +C-bug +I-unsound +O-unix
This commit is contained in:
Matthias Krüger 2023-08-20 08:34:03 +02:00 committed by GitHub
commit 7b66abe5a2
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 47 additions and 27 deletions

View File

@ -81,6 +81,10 @@ pub fn current_exe() -> io::Result<PathBuf> {
static ENV_LOCK: RwLock<()> = RwLock::new(());
pub fn env_read_lock() -> impl Drop {
ENV_LOCK.read().unwrap_or_else(PoisonError::into_inner)
}
pub struct Env {
iter: vec::IntoIter<(OsString, OsString)>,
}
@ -134,7 +138,7 @@ pub fn env() -> Env {
}
unsafe {
let _guard = ENV_LOCK.read();
let _guard = env_read_lock();
let mut result = Vec::new();
if !environ.is_null() {
while !(*environ).is_null() {
@ -168,17 +172,21 @@ pub fn env() -> Env {
pub fn getenv(k: &OsStr) -> Option<OsString> {
// environment variables with a nul byte can't be set, so their value is
// always None as well
let s = run_with_cstr(k.as_bytes(), |k| {
let _guard = ENV_LOCK.read();
Ok(unsafe { libc::getenv(k.as_ptr()) } as *const libc::c_char)
})
.ok()?;
run_with_cstr(k.as_bytes(), |k| {
let _guard = env_read_lock();
let v = unsafe { libc::getenv(k.as_ptr()) } as *const libc::c_char;
if s.is_null() {
None
if v.is_null() {
Ok(None)
} else {
Some(OsStringExt::from_vec(unsafe { CStr::from_ptr(s) }.to_bytes().to_vec()))
// SAFETY: `v` cannot be mutated while executing this line since we've a read lock
let bytes = unsafe { CStr::from_ptr(v) }.to_bytes().to_vec();
Ok(Some(OsStringExt::from_vec(bytes)))
}
})
.ok()
.flatten()
}
pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {

View File

@ -594,16 +594,21 @@ pub fn env() -> Env {
pub fn getenv(k: &OsStr) -> Option<OsString> {
// environment variables with a nul byte can't be set, so their value is
// always None as well
let s = run_with_cstr(k.as_bytes(), |k| {
run_with_cstr(k.as_bytes(), |k| {
let _guard = env_read_lock();
Ok(unsafe { libc::getenv(k.as_ptr()) } as *const libc::c_char)
})
.ok()?;
if s.is_null() {
None
let v = unsafe { libc::getenv(k.as_ptr()) } as *const libc::c_char;
if v.is_null() {
Ok(None)
} else {
Some(OsStringExt::from_vec(unsafe { CStr::from_ptr(s) }.to_bytes().to_vec()))
// SAFETY: `v` cannot be mutated while executing this line since we've a read lock
let bytes = unsafe { CStr::from_ptr(v) }.to_bytes().to_vec();
Ok(Some(OsStringExt::from_vec(bytes)))
}
})
.ok()
.flatten()
}
pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {

View File

@ -225,16 +225,23 @@ pub fn env() -> Env {
}
pub fn getenv(k: &OsStr) -> Option<OsString> {
let s = run_with_cstr(k.as_bytes(), |k| unsafe {
// environment variables with a nul byte can't be set, so their value is
// always None as well
run_with_cstr(k.as_bytes(), |k| {
let _guard = env_read_lock();
Ok(libc::getenv(k.as_ptr()) as *const libc::c_char)
})
.ok()?;
if s.is_null() {
None
let v = unsafe { libc::getenv(k.as_ptr()) } as *const libc::c_char;
if v.is_null() {
Ok(None)
} else {
Some(OsStringExt::from_vec(unsafe { CStr::from_ptr(s) }.to_bytes().to_vec()))
// SAFETY: `v` cannot be mutated while executing this line since we've a read lock
let bytes = unsafe { CStr::from_ptr(v) }.to_bytes().to_vec();
Ok(Some(OsStringExt::from_vec(bytes)))
}
})
.ok()
.flatten()
}
pub fn setenv(k: &OsStr, v: &OsStr) -> io::Result<()> {