diff --git a/src/header/stdlib/mod.rs b/src/header/stdlib/mod.rs index db708bcebce0bfd75b748ad42b875c7c261d9b01..d48b67ac387513f70b0e053018f4bb629371ac6b 100644 --- a/src/header/stdlib/mod.rs +++ b/src/header/stdlib/mod.rs @@ -324,16 +324,7 @@ pub extern "C" fn gcvt(value: c_double, ndigit: c_int, buf: *mut c_char) -> *mut } unsafe fn find_env(search: *const c_char) -> Option<(usize, *mut c_char)> { - for (i, item) in platform::inner_environ.iter().enumerate() { - let mut item = *item; - if item.is_null() { - assert_eq!( - i, - platform::inner_environ.len() - 1, - "an early null pointer in environ vector" - ); - break; - } + for (i, mut item) in platform::environ_iter().enumerate() { let mut search = search; loop { let end_of_query = *search == 0 || *search == b'=' as c_char; @@ -341,7 +332,7 @@ unsafe fn find_env(search: *const c_char) -> Option<(usize, *mut c_char)> { if *item == b'=' as c_char || end_of_query { if *item == b'=' as c_char && end_of_query { // Both keys env here - return Some((i, item.offset(1))); + return Some((i, item.add(1))); } else { break; } @@ -351,10 +342,11 @@ unsafe fn find_env(search: *const c_char) -> Option<(usize, *mut c_char)> { break; } - item = item.offset(1); - search = search.offset(1); + item = item.add(1); + search = search.add(1); } } + None } @@ -694,22 +686,33 @@ pub extern "C" fn ptsname(fildes: c_int) -> *mut c_char { unimplemented!(); } +unsafe fn put_new_env(insert: *mut c_char) { + // XXX: Another problem is that `environ` can be set to any pointer, which means there is a + // chance of a memory leak. But we can check if it was the same as before, like musl does. + if platform::environ == platform::OUR_ENVIRON.as_mut_ptr() { + *platform::OUR_ENVIRON.last_mut().unwrap() = insert; + platform::OUR_ENVIRON.push(core::ptr::null_mut()); + // Likely a no-op but is needed due to Stacked Borrows. + platform::environ = platform::OUR_ENVIRON.as_mut_ptr(); + } else { + platform::OUR_ENVIRON.clear(); + platform::OUR_ENVIRON.extend(platform::environ_iter()); + platform::OUR_ENVIRON.push(insert); + platform::OUR_ENVIRON.push(core::ptr::null_mut()); + platform::environ = platform::OUR_ENVIRON.as_mut_ptr(); + } +} + #[no_mangle] pub unsafe extern "C" fn putenv(insert: *mut c_char) -> c_int { assert_ne!(insert, ptr::null_mut(), "putenv(NULL)"); if let Some((i, _)) = find_env(insert) { - //TODO: find out why this crashes: platform::free(platform::inner_environ[i] as *mut c_void); - platform::inner_environ[i] = insert; + // XXX: The POSIX manual states that environment variables can be *set* via the `environ` + // global variable. While we can check if a pointer belongs to our allocator, or check + // `environ` against a vector which we control, it is likely not worth the effort. + platform::environ.add(i).write(insert); } else { - let i = platform::inner_environ.len() - 1; - assert_eq!( - platform::inner_environ[i], - ptr::null_mut(), - "environ did not end with null" - ); - platform::inner_environ[i] = insert; - platform::inner_environ.push(ptr::null_mut()); - platform::environ = platform::inner_environ.as_mut_ptr(); + put_new_env(insert); } 0 } @@ -850,69 +853,45 @@ pub unsafe extern "C" fn seed48(seed16v: *mut c_ushort) -> *mut c_ushort { rand48::SEED48_XSUBI.as_mut_ptr() } +unsafe fn copy_kv(existing: *mut c_char, key: *const c_char, value: *const c_char, key_len: usize, value_len: usize) { + core::ptr::copy_nonoverlapping(key, existing, key_len); + core::ptr::write(existing.add(key_len), b'=' as c_char); + core::ptr::copy_nonoverlapping(value, existing.add(key_len + 1), value_len); +} + #[no_mangle] pub unsafe extern "C" fn setenv( mut key: *const c_char, mut value: *const c_char, overwrite: c_int, ) -> c_int { - let mut key_len = 0; - while *key.offset(key_len) != 0 { - key_len += 1; - } - let mut value_len = 0; - while *value.offset(value_len) != 0 { - value_len += 1; - } + let key_len = strlen(key); + let value_len = strlen(value); - let index = if let Some((i, existing)) = find_env(key) { + if let Some((i, existing)) = find_env(key) { if overwrite == 0 { return 0; } - let mut existing_len = 0; - while *existing.offset(existing_len) != 0 { - existing_len += 1; - } + let existing_len = strlen(existing); if existing_len >= value_len { // Reuse existing element's allocation - for i in 0..=value_len { - *existing.offset(i) = *value.offset(i); - } - return 0; + core::ptr::copy_nonoverlapping(value, existing, value_len); } else { - i + // Reuse platform::environ slot, but allocate a new pointer. + let mut ptr = platform::alloc(key_len as usize + 1 + value_len as usize) as *mut c_char; + copy_kv(ptr, key, value, key_len, value_len); + platform::environ.add(i).write(ptr); } } else { - let i = platform::inner_environ.len() - 1; - assert_eq!( - platform::inner_environ[i], - ptr::null_mut(), - "environ did not end with null" - ); - platform::inner_environ.push(ptr::null_mut()); - platform::environ = platform::inner_environ.as_mut_ptr(); - i - }; + // Expand platform::environ and allocate a new pointer. + let mut ptr = platform::alloc(key_len as usize + 1 + value_len as usize) as *mut c_char; + copy_kv(ptr, key, value, key_len, value_len); + put_new_env(ptr); + } //platform::free(platform::inner_environ[index] as *mut c_void); - let mut ptr = platform::alloc(key_len as usize + 1 + value_len as usize) as *mut c_char; - platform::inner_environ[index] = ptr; - - while *key != 0 { - *ptr = *key; - ptr = ptr.offset(1); - key = key.offset(1); - } - *ptr = b'=' as c_char; - ptr = ptr.offset(1); - while *value != 0 { - *ptr = *value; - ptr = ptr.offset(1); - value = value.offset(1); - } - *ptr = 0; 0 } @@ -1174,9 +1153,19 @@ pub extern "C" fn unlockpt(fildes: c_int) -> c_int { #[no_mangle] pub unsafe extern "C" fn unsetenv(key: *const c_char) -> c_int { if let Some((i, _)) = find_env(key) { - // No need to worry about updating the pointer, this does not - // reallocate in any way. And the final null is already shifted back. - platform::inner_environ.remove(i); + if platform::environ == platform::OUR_ENVIRON.as_mut_ptr() { + // No need to worry about updating the pointer, this does not + // reallocate in any way. And the final null is already shifted back. + platform::OUR_ENVIRON.remove(i); + + // My UB paranoia. + platform::environ = platform::OUR_ENVIRON.as_mut_ptr(); + } else { + platform::OUR_ENVIRON.clear(); + platform::OUR_ENVIRON.extend(platform::environ_iter().enumerate().filter(|&(j, _)| j != i).map(|(_, v)| v)); + platform::OUR_ENVIRON.push(core::ptr::null_mut()); + platform::environ = platform::OUR_ENVIRON.as_mut_ptr(); + } } 0 } diff --git a/src/platform/mod.rs b/src/platform/mod.rs index 35a35e4f70e5fad88fcaca2ce623d559d57aeced..890f83245dbfbca3fa55b45acd3ceee698447687 100644 --- a/src/platform/mod.rs +++ b/src/platform/mod.rs @@ -54,8 +54,24 @@ pub static mut program_invocation_short_name: *mut c_char = ptr::null_mut(); #[allow(non_upper_case_globals)] #[no_mangle] pub static mut environ: *mut *mut c_char = ptr::null_mut(); -#[allow(non_upper_case_globals)] -pub static mut inner_environ: Vec<*mut c_char> = Vec::new(); + +pub static mut OUR_ENVIRON: Vec<*mut c_char> = Vec::new(); + +pub fn environ_iter() -> impl Iterator<Item = *mut c_char> + 'static { + unsafe { + let mut ptrs = environ; + + core::iter::from_fn(move || { + let ptr = ptrs.read(); + if ptr.is_null() { + None + } else { + ptrs = ptrs.add(1); + Some(ptr) + } + }) + } +} pub trait WriteByte: fmt::Write { fn write_u8(&mut self, byte: u8) -> fmt::Result; diff --git a/src/start.rs b/src/start.rs index 95d08373089092fb9797ff5299432f35b8b51ef9..cee25f33d96dba8dc5ff942c4a59ba4479ad93c9 100644 --- a/src/start.rs +++ b/src/start.rs @@ -162,8 +162,8 @@ pub unsafe extern "C" fn relibc_start(sp: &'static Stack) -> ! { while !(*envp.add(len)).is_null() { len += 1; } - platform::inner_environ = copy_string_array(envp, len); - platform::environ = platform::inner_environ.as_mut_ptr(); + platform::OUR_ENVIRON = copy_string_array(envp, len); + platform::environ = platform::OUR_ENVIRON.as_mut_ptr(); // Setup signal stack, otherwise we cannot handle any signals besides SIG_IGN/SIG_DFL behavior. setup_sigstack();