Make various library loading functions unsafe

src/lib.rs

@@ -26,8 +26,8 @@
 //! extern crate libloading as lib;
 //!
 //! fn call_dynamic() -> Result<u32, Box<dyn std::error::Error>> {
-//!     let lib = lib::Library::new("/path/to/liblibrary.so")?;
 //!     unsafe {
+//!         let lib = lib::Library::new("/path/to/liblibrary.so")?;
 //!         let func: lib::Symbol<unsafe extern fn() -> u32> = lib.get(b"my_func")?;
 //!         Ok(func())
 //!     }
@@ -74,6 +74,12 @@ impl Library {
     /// * Absolute path to the library;
     /// * Relative (to the current working directory) path to the library.
     ///
+    /// # Safety
+    ///
+    /// When a library is loaded initializers contained within the library are executed. For the
+    /// purposes of soundness, execution of these initializers is conceptually the same calling a
+    /// FFI function and may impose whatever requirements on the caller.
+    ///
     /// # Thread-safety
     ///
     /// The implementation strives to be as MT-safe as sanely possible, however due to certain
@@ -123,11 +129,13 @@ impl Library {
     /// ```no_run
     /// # use ::libloading::Library;
     /// // Any of the following are valid.
-    /// let _ = Library::new("/path/to/awesome.module").unwrap();
-    /// let _ = Library::new("../awesome.module").unwrap();
-    /// let _ = Library::new("libsomelib.so.1").unwrap();
+    /// unsafe {
+    ///     let _ = Library::new("/path/to/awesome.module").unwrap();
+    ///     let _ = Library::new("../awesome.module").unwrap();
+    ///     let _ = Library::new("libsomelib.so.1").unwrap();
+    /// }
     /// ```
-    pub fn new<P: AsRef<OsStr>>(filename: P) -> Result<Library, Error> {
+    pub unsafe fn new<P: AsRef<OsStr>>(filename: P) -> Result<Library, Error> {
         imp::Library::new(filename).map(From::from)
     }
 
@@ -162,14 +170,18 @@ impl Library {
     ///
     /// ```no_run
     /// # use ::libloading::Library;
-    /// let lib = Library::new("/path/to/awesome.module").unwrap();
+    /// let lib = unsafe {
+    ///     Library::new("/path/to/awesome.module").unwrap()
+    /// };
     /// ```
     ///
     /// Loading and using a function looks like this:
     ///
     /// ```no_run
     /// # use ::libloading::{Library, Symbol};
-    /// # let lib = Library::new("/path/to/awesome.module").unwrap();
+    /// # let lib = unsafe {
+    /// #     Library::new("/path/to/awesome.module").unwrap()
+    /// # };
     /// unsafe {
     ///     let awesome_function: Symbol<unsafe extern fn(f64) -> f64> =
     ///         lib.get(b"awesome_function\0").unwrap();
@@ -181,7 +193,7 @@ impl Library {
     ///
     /// ```no_run
     /// # use ::libloading::{Library, Symbol};
-    /// # let lib = Library::new("/path/to/awesome.module").unwrap();
+    /// # let lib = unsafe { Library::new("/path/to/awesome.module").unwrap() };
     /// unsafe {
     ///     let awesome_variable: Symbol<*mut f64> = lib.get(b"awesome_variable\0").unwrap();
     ///     **awesome_variable = 42.0;
@@ -257,8 +269,8 @@ impl<'lib, T> Symbol<'lib, T> {
     ///
     /// ```no_run
     /// # use ::libloading::{Library, Symbol};
-    /// let lib = Library::new("/path/to/awesome.module").unwrap();
     /// unsafe {
+    ///     let lib = Library::new("/path/to/awesome.module").unwrap();
     ///     let symbol: Symbol<*mut u32> = lib.get(b"symbol\0").unwrap();
     ///     let symbol = symbol.into_raw();
     /// }
@@ -281,8 +293,8 @@ impl<'lib, T> Symbol<'lib, T> {
     ///
     /// ```no_run
     /// # use ::libloading::{Library, Symbol};
-    /// let lib = Library::new("/path/to/awesome.module").unwrap();
     /// unsafe {
+    ///     let lib = Library::new("/path/to/awesome.module").unwrap();
     ///     let symbol: Symbol<*mut u32> = lib.get(b"symbol\0").unwrap();
     ///     let symbol = symbol.into_raw();
     ///     let symbol = Symbol::from_raw(symbol, &lib);
@@ -303,8 +315,8 @@ impl<'lib, T> Symbol<'lib, Option<T>> {
     ///
     /// ```no_run
     /// # use ::libloading::{Library, Symbol};
-    /// let lib = Library::new("/path/to/awesome.module").unwrap();
     /// unsafe {
+    ///     let lib = Library::new("/path/to/awesome.module").unwrap();
     ///     let symbol: Symbol<Option<*mut u32>> = lib.get(b"symbol\0").unwrap();
     ///     let symbol: Symbol<*mut u32> = symbol.lift_option().expect("static is not null");
     /// }
@@ -356,7 +368,9 @@ unsafe impl<'lib, T: Sync> Sync for Symbol<'lib, T> {}
 /// use libloading::{Library, library_filename};
 /// // Will attempt to load `libLLVM.so` on Linux, `libLLVM.dylib` on macOS and `LLVM.dll` on
 /// // Windows.
-/// let library = Library::new(library_filename("LLVM"));
+/// let library = unsafe {
+///     Library::new(library_filename("LLVM"))
+/// };
 /// ```
 pub fn library_filename<S: AsRef<OsStr>>(name: S) -> OsString {
     let name = name.as_ref();

src/os/unix/mod.rs

@@ -106,8 +106,14 @@ impl Library {
     /// This is equivalent to [`Library::open`]`(filename, RTLD_LAZY | RTLD_LOCAL)`.
     ///
     /// [path separator]: std::path::MAIN_SEPARATOR
+    ///
+    /// # Safety
+    ///
+    /// When a library is loaded initializers contained within the library are executed. For the
+    /// purposes of soundness, execution of these initializers is conceptually the same calling a
+    /// FFI function and may impose whatever requirements on the caller.
     #[inline]
-    pub fn new<P: AsRef<OsStr>>(filename: P) -> Result<Library, crate::Error> {
+    pub unsafe fn new<P: AsRef<OsStr>>(filename: P) -> Result<Library, crate::Error> {
         Library::open(Some(filename), RTLD_LAZY | RTLD_LOCAL)
     }
 
@@ -129,7 +135,10 @@ impl Library {
     /// [`os::windows::Library::this`]: crate::os::windows::Library::this
     #[inline]
     pub fn this() -> Library {
-        Library::open(None::<&OsStr>, RTLD_LAZY | RTLD_LOCAL).expect("this should never fail")
+        unsafe {
+            // SAFE: this obtains a handle to an existing, loaded module, no danger in
+            Library::open(None::<&OsStr>, RTLD_LAZY | RTLD_LOCAL).expect("this should never fail")
+        }
     }
 
     /// Find and load an executable object file (shared library).
@@ -138,22 +147,25 @@ impl Library {
     /// when the `filename` is `None`. Otherwise see [`Library::new`].
     ///
     /// Corresponds to `dlopen(filename, flags)`.
-    pub fn open<P>(filename: Option<P>, flags: raw::c_int) -> Result<Library, crate::Error>
+    ///
+    /// # Safety
+    ///
+    /// When a library is loaded initializers contained within the library are executed. For the
+    /// purposes of soundness, execution of these initializers is conceptually the same calling a
+    /// FFI function and may impose whatever requirements on the caller.
+    pub unsafe fn open<P>(filename: Option<P>, flags: raw::c_int) -> Result<Library, crate::Error>
     where P: AsRef<OsStr> {
         let filename = match filename {
             None => None,
             Some(ref f) => Some(cstr_cow_from_bytes(f.as_ref().as_bytes())?),
         };
         with_dlerror(|desc| crate::Error::DlOpen { desc }, move || {
-            let result = unsafe {
-                let r = dlopen(match filename {
-                    None => ptr::null(),
-                    Some(ref f) => f.as_ptr()
-                }, flags);
-                // ensure filename lives until dlopen completes
-                drop(filename);
-                r
-            };
+            let result = dlopen(match filename {
+                None => ptr::null(),
+                Some(ref f) => f.as_ptr()
+            }, flags);
+            // ensure filename lives until dlopen completes
+            drop(filename);
             if result.is_null() {
                 None
             } else {

src/os/windows/mod.rs

@@ -91,12 +91,18 @@ impl Library {
     /// This is equivalent to [`Library::load_with_flags`]`(filename, 0)`.
     ///
     /// [msdn]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryw#remarks
+    ///
+    /// # Safety
+    ///
+    /// When a library is loaded initializers contained within the library are executed. For the
+    /// purposes of soundness, execution of these initializers is conceptually the same calling a
+    /// FFI function and may impose whatever requirements on the caller.
     #[inline]
-    pub fn new<P: AsRef<OsStr>>(filename: P) -> Result<Library, crate::Error> {
+    pub unsafe fn new<P: AsRef<OsStr>>(filename: P) -> Result<Library, crate::Error> {
         Library::load_with_flags(filename, 0)
     }
 
-    /// Load the `Library` representing the original program executable.
+    /// Get the `Library` representing the original program executable.
     ///
     /// Note that behaviour of `Library` loaded with this method is different from
     /// Libraries loaded with [`os::unix::Library::this`]. For more information refer to [MSDN].
@@ -119,7 +125,7 @@ impl Library {
         }
     }
 
-    /// Load a module that is already loaded by the program.
+    /// Get a module that is already loaded by the program.
     ///
     /// This function returns a `Library` corresponding to a module with the given name that is
     /// already mapped into the address space of the process. If the module isn't found an error is
@@ -166,16 +172,21 @@ impl Library {
     /// Corresponds to `LoadLibraryExW(filename, reserved: NULL, flags)`.
     ///
     /// [flags]: https://docs.microsoft.com/en-us/windows/win32/api/libloaderapi/nf-libloaderapi-loadlibraryexw#parameters
-    pub fn load_with_flags<P: AsRef<OsStr>>(filename: P, flags: DWORD) -> Result<Library, crate::Error> {
+    ///
+    /// # Safety
+    ///
+    /// When a library is loaded initializers contained within the library are executed. For the
+    /// purposes of soundness, execution of these initializers is conceptually the same calling a
+    /// FFI function and may impose whatever requirements on the caller.
+    pub unsafe fn load_with_flags<P: AsRef<OsStr>>(filename: P, flags: DWORD) -> Result<Library, crate::Error> {
         let wide_filename: Vec<u16> = filename.as_ref().encode_wide().chain(Some(0)).collect();
         let _guard = ErrorModeGuard::new();
 
         let ret = with_get_last_error(|source| crate::Error::LoadLibraryExW { source }, || {
             // Make sure no winapi calls as a result of drop happen inside this closure, because
             // otherwise that might change the return value of the GetLastError.
-            let handle = unsafe {
-                libloaderapi::LoadLibraryExW(wide_filename.as_ptr(), std::ptr::null_mut(), flags)
-            };
+            let handle =
+                libloaderapi::LoadLibraryExW(wide_filename.as_ptr(), std::ptr::null_mut(), flags);
             if handle.is_null()  {
                 None
             } else {

tests/functions.rs

@@ -31,8 +31,8 @@ fn make_helpers() {
 #[test]
 fn test_id_u32() {
     make_helpers();
-    let lib = Library::new(LIBPATH).unwrap();
     unsafe {
+        let lib = Library::new(LIBPATH).unwrap();
         let f: Symbol<unsafe extern fn(u32) -> u32> = lib.get(b"test_identity_u32\0").unwrap();
         assert_eq!(42, f(42));
     }
@@ -50,8 +50,8 @@ struct S {
 #[test]
 fn test_id_struct() {
     make_helpers();
-    let lib = Library::new(LIBPATH).unwrap();
     unsafe {
+        let lib = Library::new(LIBPATH).unwrap();
         let f: Symbol<unsafe extern fn(S) -> S> = lib.get(b"test_identity_struct\0").unwrap();
         assert_eq!(S { a: 1, b: 2, c: 3, d: 4 }, f(S { a: 1, b: 2, c: 3, d: 4 }));
     }
@@ -60,8 +60,8 @@ fn test_id_struct() {
 #[test]
 fn test_0_no_0() {
     make_helpers();
-    let lib = Library::new(LIBPATH).unwrap();
     unsafe {
+        let lib = Library::new(LIBPATH).unwrap();
         let f: Symbol<unsafe extern fn(S) -> S> = lib.get(b"test_identity_struct\0").unwrap();
         let f2: Symbol<unsafe extern fn(S) -> S> = lib.get(b"test_identity_struct").unwrap();
         assert_eq!(*f, *f2);
@@ -70,14 +70,16 @@ fn test_0_no_0() {
 
 #[test]
 fn wrong_name_fails() {
-    Library::new("target/this_location_is_definitely_non existent:^~").err().unwrap();
+    unsafe {
+        Library::new("target/this_location_is_definitely_non existent:^~").err().unwrap();
+    }
 }
 
 #[test]
 fn missing_symbol_fails() {
     make_helpers();
-    let lib = Library::new(LIBPATH).unwrap();
     unsafe {
+        let lib = Library::new(LIBPATH).unwrap();
         lib.get::<*mut ()>(b"test_does_not_exist").err().unwrap();
         lib.get::<*mut ()>(b"test_does_not_exist\0").err().unwrap();
     }
@@ -86,8 +88,8 @@ fn missing_symbol_fails() {
 #[test]
 fn interior_null_fails() {
     make_helpers();
-    let lib = Library::new(LIBPATH).unwrap();
     unsafe {
+        let lib = Library::new(LIBPATH).unwrap();
         lib.get::<*mut ()>(b"test_does\0_not_exist").err().unwrap();
         lib.get::<*mut ()>(b"test\0_does_not_exist\0").err().unwrap();
     }
@@ -96,8 +98,8 @@ fn interior_null_fails() {
 #[test]
 fn test_incompatible_type() {
     make_helpers();
-    let lib = Library::new(LIBPATH).unwrap();
     unsafe {
+        let lib = Library::new(LIBPATH).unwrap();
         assert!(match lib.get::<()>(b"test_identity_u32\0") {
            Err(libloading::Error::IncompatibleSize) => true,
            _ => false,
@@ -111,8 +113,8 @@ fn test_incompatible_type_named_fn() {
     unsafe fn get<'a, T>(l: &'a Library, _: T) -> Result<Symbol<'a, T>, libloading::Error> {
         l.get::<T>(b"test_identity_u32\0")
     }
-    let lib = Library::new(LIBPATH).unwrap();
     unsafe {
+        let lib = Library::new(LIBPATH).unwrap();
         assert!(match get(&lib, test_incompatible_type_named_fn) {
            Err(libloading::Error::IncompatibleSize) => true,
            _ => false,
@@ -123,8 +125,8 @@ fn test_incompatible_type_named_fn() {
 #[test]
 fn test_static_u32() {
     make_helpers();
-    let lib = Library::new(LIBPATH).unwrap();
     unsafe {
+        let lib = Library::new(LIBPATH).unwrap();
         let var: Symbol<*mut u32> = lib.get(b"TEST_STATIC_U32\0").unwrap();
         **var = 42;
         let help: Symbol<unsafe extern fn() -> u32> = lib.get(b"test_get_static_u32\0").unwrap();
@@ -135,8 +137,8 @@ fn test_static_u32() {
 #[test]
 fn test_static_ptr() {
     make_helpers();
-    let lib = Library::new(LIBPATH).unwrap();
     unsafe {
+        let lib = Library::new(LIBPATH).unwrap();
         let var: Symbol<*mut *mut ()> = lib.get(b"TEST_STATIC_PTR\0").unwrap();
         **var = *var as *mut _;
         let works: Symbol<unsafe extern fn() -> bool> =
@@ -174,13 +176,12 @@ fn manual_close_many_times() {
 fn library_this_get() {
     use libloading::os::unix::Library;
     make_helpers();
-    let _lib = Library::new(LIBPATH).unwrap();
-    let this = Library::this();
     // SAFE: functions are never called
     unsafe {
+        let _lib = Library::new(LIBPATH).unwrap();
+        let this = Library::this();
         // Library we loaded in `_lib` (should be RTLD_LOCAL).
-        // FIXME: inconsistent behaviour between macos and other posix systems
-        // assert!(this.get::<unsafe extern "C" fn()>(b"test_identity_u32").is_err());
+        assert!(this.get::<unsafe extern "C" fn()>(b"test_identity_u32").is_err());
         // Something obscure from libc...
         assert!(this.get::<unsafe extern "C" fn()>(b"freopen").is_ok());
     }
@@ -191,10 +192,11 @@ fn library_this_get() {
 fn library_this() {
     use libloading::os::windows::Library;
     make_helpers();
-    let _lib = Library::new(LIBPATH).unwrap();
-    let this = Library::this().expect("this library");
-    // SAFE: functions are never called
     unsafe {
+        // SAFE: well-known library without initializers is loaded.
+        let _lib = Library::new(LIBPATH).unwrap();
+        let this = Library::this().expect("this library");
+        // SAFE: functions are never called.
         // Library we loaded in `_lib`.
         assert!(this.get::<unsafe extern "C" fn()>(b"test_identity_u32").is_err());
         // Something "obscure" from kernel32...
@@ -209,11 +211,9 @@ fn works_getlasterror() {
     use winapi::shared::minwindef::DWORD;
     use libloading::os::windows::{Library, Symbol};
 
-    let lib = Library::new("kernel32.dll").unwrap();
-    let gle: Symbol<unsafe extern "system" fn() -> DWORD> = unsafe {
-        lib.get(b"GetLastError").unwrap()
-    };
     unsafe {
+        let lib = Library::new("kernel32.dll").unwrap();
+        let gle: Symbol<unsafe extern "system" fn() -> DWORD> = lib.get(b"GetLastError").unwrap();
         errhandlingapi::SetLastError(42);
         assert_eq!(errhandlingapi::GetLastError(), gle())
     }
@@ -226,11 +226,9 @@ fn works_getlasterror0() {
     use winapi::shared::minwindef::DWORD;
     use libloading::os::windows::{Library, Symbol};
 
-    let lib = Library::new("kernel32.dll").unwrap();
-    let gle: Symbol<unsafe extern "system" fn() -> DWORD> = unsafe {
-        lib.get(b"GetLastError\0").unwrap()
-    };
     unsafe {
+        let lib = Library::new("kernel32.dll").unwrap();
+        let gle: Symbol<unsafe extern "system" fn() -> DWORD> = lib.get(b"GetLastError\0").unwrap();
         errhandlingapi::SetLastError(42);
         assert_eq!(errhandlingapi::GetLastError(), gle())
     }
@@ -250,8 +248,9 @@ fn library_open_already_loaded() {
         _ => false,
     });
 
-    let _lib = Library::new(LIBPATH).unwrap();
-
-    // Loaded now.
-    assert!(Library::open_already_loaded(LIBPATH).is_ok());
+    unsafe {
+        let _lib = Library::new(LIBPATH).unwrap();
+        // Loaded now.
+        assert!(Library::open_already_loaded(LIBPATH).is_ok());
+    }
 }

tests/windows.rs

@@ -15,12 +15,16 @@ use std::ffi::CStr;
 
 #[cfg(target_arch="x86")]
 fn load_ordinal_lib() -> Library {
-    Library::new("tests/nagisa32.dll").expect("nagisa32.dll")
+    unsafe {
+        Library::new("tests/nagisa32.dll").expect("nagisa32.dll")
+    }
 }
 
 #[cfg(target_arch="x86_64")]
 fn load_ordinal_lib() -> Library {
-    Library::new("tests/nagisa64.dll").expect("nagisa64.dll")
+    unsafe {
+        Library::new("tests/nagisa64.dll").expect("nagisa64.dll")
+    }
 }
 
 #[cfg(any(target_arch="x86", target_arch="x86_64"))]
@@ -47,10 +51,14 @@ fn test_ordinal_missing_fails() {
 
 #[test]
 fn test_new_kernel23() {
-    Library::new("kernel23").err().unwrap();
+    unsafe {
+        Library::new("kernel23").err().unwrap();
+    }
 }
 
 #[test]
 fn test_new_kernel32_no_ext() {
-    Library::new("kernel32").unwrap();
+    unsafe {
+        Library::new("kernel32").unwrap();
+    }
 }