Skip to content

Commit

Permalink
Improve memory safety of Weak::new().
Browse files Browse the repository at this point in the history
Do null terminator checking more completely and at compile time.
  • Loading branch information
briansmith committed May 20, 2024
1 parent 07071c7 commit bf7c4b1
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 7 deletions.
3 changes: 2 additions & 1 deletion src/netbsd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ type GetRandomFn = unsafe extern "C" fn(*mut u8, libc::size_t, libc::c_uint) ->

pub fn getrandom_inner(dest: &mut [MaybeUninit<u8>]) -> Result<(), Error> {
// getrandom(2) was introduced in NetBSD 10.0
static GETRANDOM: Weak = unsafe { Weak::new("getrandom\0") };
static GETRANDOM_NAME: cstr::Ref = cstr::unwrap_const_from_bytes_with_nul(b"getrandom\0");
static GETRANDOM: Weak = Weak::new(GETRANDOM_NAME);
if let Some(fptr) = GETRANDOM.ptr() {
let func: GetRandomFn = unsafe { core::mem::transmute(fptr) };
return sys_fill_exact(dest, |buf| unsafe {
Expand Down
10 changes: 4 additions & 6 deletions src/util_libc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ pub fn sys_fill_exact(
// https://github.com/rust-lang/rust/blob/1.61.0/library/std/src/sys/unix/weak.rs#L84
// except that the caller must manually cast self.ptr() to a function pointer.
pub struct Weak {
name: &'static str,
name: cstr::Ref,
addr: AtomicPtr<c_void>,
}

Expand All @@ -98,9 +98,8 @@ impl Weak {
// TODO: Replace with core::ptr::invalid_mut(1) when that is stable.
const UNINIT: *mut c_void = 1 as *mut c_void;

// Construct a binding to a C function with a given name. This function is
// unsafe because `name` _must_ be null terminated.
pub const unsafe fn new(name: &'static str) -> Self {
// Construct a binding to a C function with a given name.
pub const fn new(name: cstr::Ref) -> Self {
Self {
name,
addr: AtomicPtr::new(Self::UNINIT),
Expand All @@ -120,8 +119,7 @@ impl Weak {
// the use of non-Relaxed operations is probably unnecessary.
match self.addr.load(Ordering::Relaxed) {
Self::UNINIT => {
// XXX/FIXME: Unchecked UTF-8-to-c_char cast.
let symbol = self.name.as_ptr().cast::<libc::c_char>();
let symbol = self.name.as_ptr();
let addr = unsafe { libc::dlsym(libc::RTLD_DEFAULT, symbol) };
// Synchronizes with the Acquire fence below
self.addr.store(addr, Ordering::Release);
Expand Down

0 comments on commit bf7c4b1

Please sign in to comment.