Skip to content

Commit

Permalink
Use AtomicPtr instead of AtomicUsize for Weak
Browse files Browse the repository at this point in the history
This allows Strict Provenance to work properly, fixing #262. It also
now matches what `libstd` does:
https://github.com/rust-lang/rust/blob/9f7e997c8bc3cacd2ab4eb75e63cb5fa9279c7b0/library/std/src/sys/unix/weak.rs#L85-L141

Also, while reading the `libstd` code, I noticed that they use an
`Acquire` fence and `Release` store as the returned pointer should
have "consume" semantics. I changed our code to do something
slightly stronger (Acquire load and Release store) for consistancy.

Signed-off-by: Joe Richey <joerichey@google.com>
  • Loading branch information
josephlr committed Jun 7, 2022
1 parent 9e2c896 commit 27f4f82
Showing 1 changed file with 36 additions and 12 deletions.
48 changes: 36 additions & 12 deletions src/util_libc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,13 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.
#![allow(dead_code)]
use crate::{util::LazyUsize, Error};
use core::{num::NonZeroU32, ptr::NonNull};
use crate::Error;
use core::{
num::NonZeroU32,
ptr::NonNull,
sync::atomic::{AtomicPtr, Ordering},
};
use libc::c_void;

cfg_if! {
if #[cfg(any(target_os = "netbsd", target_os = "openbsd", target_os = "android"))] {
Expand Down Expand Up @@ -76,29 +81,48 @@ pub fn sys_fill_exact(

// A "weak" binding to a C function that may or may not be present at runtime.
// Used for supporting newer OS features while still building on older systems.
// F must be a function pointer of type `unsafe extern "C" fn`. Based off of the
// weak! macro in libstd.
// Based off of the DlsymWeak struct in libstd (src/sys/unix/weak.rs), except
// that the caller must cast self.ptr() to a function pointer.
pub struct Weak {
name: &'static str,
addr: LazyUsize,
addr: AtomicPtr<c_void>,
}

impl Weak {
// A non-null pointer value which indicates we are uninitialized. This
// constant must respect Strict Provenance should not be a valid address
// of a function pointer. However, if libc::dlsym does return UNINIT, there
// will not be undefined behavior. libc::dlsym will just be called each
// time ptr() is called. This would be inefficient, but correct.
// TODO: Replace with core::ptr::invalid_mut(1) when that is stable.
const UNINIT: *mut c_void = NonNull::dangling().as_ptr();

// 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 {
Self {
name,
addr: LazyUsize::new(),
addr: AtomicPtr::new(Self::UNINIT),
}
}

// Return a function pointer if present at runtime. Otherwise, return null.
pub fn ptr(&self) -> Option<NonNull<libc::c_void>> {
let addr = self.addr.unsync_init(|| unsafe {
libc::dlsym(libc::RTLD_DEFAULT, self.name.as_ptr() as *const _) as usize
});
NonNull::new(addr as *mut _)
// Return the address of a function if present at runtime. Otherwise,
// return null. Multiple callers can call ptr() concurrently. It will
// always return _some_ value returned by libc::dlsym. However, the
// dlsym function may be called multiple times.
pub fn ptr(&self) -> Option<NonNull<c_void>> {
// Despite having only a single atomic variable (self.addr), we still
// need a "consume" ordering as we will generally be reading though
// this value (by calling the function we dlsym'ed). Rust lacks this
// ordering, so we have to go with the next strongest: Acquire/Release.
// As noted in libstd, this might be unnecessary.
let mut addr = self.addr.load(Ordering::Acquire);
if addr == Self::UNINIT {
let symbol = self.name.as_ptr() as *const _;
addr = unsafe { libc::dlsym(libc::RTLD_DEFAULT, symbol) };
self.addr.store(addr, Ordering::Release);
}
NonNull::new(addr)
}
}

Expand Down

0 comments on commit 27f4f82

Please sign in to comment.