Skip to content

Commit c82a522

Browse files
josephlrfbstj
andauthored
Use AtomicPtr instead of AtomicUsize for Weak (#263)
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. As this doesn't yet exist in Rust, we instead do exactly what `libstd` does, which means: - Relaxed Load - Release Store - Acquire fence when returning pointer Signed-off-by: Joe Richey <joerichey@google.com> Co-authored-by: Joe ST <joe@fbstj.net>
1 parent 9e2c896 commit c82a522

File tree

1 file changed

+45
-12
lines changed

1 file changed

+45
-12
lines changed

src/util_libc.rs

Lines changed: 45 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,13 @@
66
// option. This file may not be copied, modified, or distributed
77
// except according to those terms.
88
#![allow(dead_code)]
9-
use crate::{util::LazyUsize, Error};
10-
use core::{num::NonZeroU32, ptr::NonNull};
9+
use crate::Error;
10+
use core::{
11+
num::NonZeroU32,
12+
ptr::NonNull,
13+
sync::atomic::{fence, AtomicPtr, Ordering},
14+
};
15+
use libc::c_void;
1116

1217
cfg_if! {
1318
if #[cfg(any(target_os = "netbsd", target_os = "openbsd", target_os = "android"))] {
@@ -76,29 +81,57 @@ pub fn sys_fill_exact(
7681

7782
// A "weak" binding to a C function that may or may not be present at runtime.
7883
// Used for supporting newer OS features while still building on older systems.
79-
// F must be a function pointer of type `unsafe extern "C" fn`. Based off of the
80-
// weak! macro in libstd.
84+
// Based off of the DlsymWeak struct in libstd:
85+
// https://github.com/rust-lang/rust/blob/1.61.0/library/std/src/sys/unix/weak.rs#L84
86+
// except that the caller must manually cast self.ptr() to a function pointer.
8187
pub struct Weak {
8288
name: &'static str,
83-
addr: LazyUsize,
89+
addr: AtomicPtr<c_void>,
8490
}
8591

8692
impl Weak {
93+
// A non-null pointer value which indicates we are uninitialized. This
94+
// constant should ideally not be a valid address of a function pointer.
95+
// However, if by chance libc::dlsym does return UNINIT, there will not
96+
// be undefined behavior. libc::dlsym will just be called each time ptr()
97+
// is called. This would be inefficient, but correct.
98+
// TODO: Replace with core::ptr::invalid_mut(1) when that is stable.
99+
const UNINIT: *mut c_void = 1 as *mut c_void;
100+
87101
// Construct a binding to a C function with a given name. This function is
88102
// unsafe because `name` _must_ be null terminated.
89103
pub const unsafe fn new(name: &'static str) -> Self {
90104
Self {
91105
name,
92-
addr: LazyUsize::new(),
106+
addr: AtomicPtr::new(Self::UNINIT),
93107
}
94108
}
95109

96-
// Return a function pointer if present at runtime. Otherwise, return null.
97-
pub fn ptr(&self) -> Option<NonNull<libc::c_void>> {
98-
let addr = self.addr.unsync_init(|| unsafe {
99-
libc::dlsym(libc::RTLD_DEFAULT, self.name.as_ptr() as *const _) as usize
100-
});
101-
NonNull::new(addr as *mut _)
110+
// Return the address of a function if present at runtime. Otherwise,
111+
// return None. Multiple callers can call ptr() concurrently. It will
112+
// always return _some_ value returned by libc::dlsym. However, the
113+
// dlsym function may be called multiple times.
114+
pub fn ptr(&self) -> Option<NonNull<c_void>> {
115+
// Despite having only a single atomic variable (self.addr), we still
116+
// cannot always use Ordering::Relaxed, as we need to make sure a
117+
// successful call to dlsym() is "ordered before" any data read through
118+
// the returned pointer (which occurs when the function is called).
119+
// Our implementation mirrors that of the one in libstd, meaning that
120+
// the use of non-Relaxed operations is probably unnecessary.
121+
match self.addr.load(Ordering::Relaxed) {
122+
Self::UNINIT => {
123+
let symbol = self.name.as_ptr() as *const _;
124+
let addr = unsafe { libc::dlsym(libc::RTLD_DEFAULT, symbol) };
125+
// Synchronizes with the Acquire fence below
126+
self.addr.store(addr, Ordering::Release);
127+
NonNull::new(addr)
128+
}
129+
addr => {
130+
let func = NonNull::new(addr)?;
131+
fence(Ordering::Acquire);
132+
Some(func)
133+
}
134+
}
102135
}
103136
}
104137

0 commit comments

Comments
 (0)