std: improve safety of dlsym!

Author: joboet

library/std/src/lib.rs

@@ -350,6 +350,7 @@
 #![feature(float_gamma)]
 #![feature(float_minimum_maximum)]
 #![feature(fmt_internals)]
+#![feature(fn_ptr_trait)]
 #![feature(generic_atomic)]
 #![feature(hasher_prefixfree_extras)]
 #![feature(hashmap_internals)]

library/std/src/sys/pal/unix/weak.rs

@@ -23,7 +23,7 @@
 #![forbid(unsafe_op_in_unsafe_fn)]
 
 use crate::ffi::{CStr, c_char, c_void};
-use crate::marker::PhantomData;
+use crate::marker::{FnPtr, PhantomData};
 use crate::sync::atomic::{Atomic, AtomicPtr, Ordering};
 use crate::{mem, ptr};
 
@@ -76,8 +76,18 @@ pub(crate) macro dlsym {
         #[link_name = $sym:expr]
         fn $name:ident($($param:ident : $t:ty),* $(,)?) -> $ret:ty;
     ) => (
-        static DLSYM: DlsymWeak<unsafe extern "C" fn($($t),*) -> $ret> =
-            DlsymWeak::new(concat!($sym, '\0'));
+        static DLSYM: DlsymWeak<unsafe extern "C" fn($($t),*) -> $ret> = {
+            let Ok(name) = CStr::from_bytes_with_nul(concat!($sym, '\0').as_bytes()) else {
+                panic!("symbol name may not contain NUL")
+            };
+
+            // SAFETY: Whoever calls the function pointer returned by `get()`
+            // is responsible for ensuring that the signature is correct. Just
+            // like with extern blocks, this is syntactically enforced by making
+            // the function pointer be unsafe.
+            unsafe { DlsymWeak::new(name) }
+        };
+
         let $name = &DLSYM;
     )
 }
@@ -90,12 +100,13 @@ pub(crate) struct DlsymWeak<F> {
     _marker: PhantomData<F>,
 }
 
-impl<F> DlsymWeak<F> {
-    pub(crate) const fn new(name: &'static str) -> Self {
-        let Ok(name) = CStr::from_bytes_with_nul(name.as_bytes()) else {
-            panic!("not a nul-terminated string")
-        };
-
+impl<F: FnPtr> DlsymWeak<F> {
+    /// # Safety
+    ///
+    /// If the signature of `F` does not match the signature of the symbol (if
+    /// it exists), calling the function pointer returned by `get()` is
+    /// undefined behaviour.
+    pub(crate) const unsafe fn new(name: &'static CStr) -> Self {
         DlsymWeak {
             name: name.as_ptr(),
             func: AtomicPtr::new(ptr::without_provenance_mut(1)),
@@ -125,26 +136,32 @@ impl<F> DlsymWeak<F> {
         match self.func.load(Ordering::Acquire) {
             func if func.addr() == 1 => self.initialize(),
             func if func.is_null() => None,
+            // SAFETY:
+            // `func` is not null and `F` implements `FnPtr`, thus this
+            // transmutation is well-defined. It is the responsibility of the
+            // creator of this `DlsymWeak` to ensure that calling the resulting
+            // function pointer does not result in undefined behaviour (though
+            // the `dlsym!` macro delegates this responsibility to the caller
+            // of the function by using `unsafe` function pointers).
+            // FIXME: use `transmute` once it stops complaining about generics.
             func => Some(unsafe { mem::transmute_copy::<*mut c_void, F>(&func) }),
         }
     }
 
     // Cold because it should only happen during first-time initialization.
     #[cold]
     fn initialize(&self) -> Option<F> {
-        const {
-            if size_of::<F>() != size_of::<*mut libc::c_void>() {
-                panic!("not a function pointer")
-            }
-        }
-
+        // SAFETY: `self.name` was created from a `&'static CStr` and is
+        // therefore a valid C string pointer.
         let val = unsafe { libc::dlsym(libc::RTLD_DEFAULT, self.name) };
         // This synchronizes with the acquire load in `get`.
         self.func.store(val, Ordering::Release);
 
         if val.is_null() {
             None
         } else {
+            // SAFETY: see the comment in `get`.
+            // FIXME: use `transmute` once it stops complaining about generics.
             Some(unsafe { mem::transmute_copy::<*mut libc::c_void, F>(&val) })
         }
     }