From 9c5c6e929330a6a8052bdc972a6bb8d519df91ba Mon Sep 17 00:00:00 2001
From: "Joshua M. Clulow" <jmc@oxide.computer>
Date: Wed, 15 Jul 2020 16:55:28 -0700
Subject: [PATCH] consult dlerror() only if a dl*() call fails

The string returned from dlerror() is purely diagnostic and should not
itself be used to determine whether a previous call to dlopen() or
dlsym() has failed.  Those functions are documented with specific return
values that signal failure; i.e., returning NULL.

If we assume a non-NULL return from dlerror() means the prior dlsym()
call failed, we are vulnerable to a race with another thread outside of
Rust control concurrently inducing dynamic linking operations.  This
manifests on illumos systems with an intermittent spurious failure from
rustc:

    error: ld.so.1: rustc: fatal: _ex_unwind: can't find symbol

The illumos libc checks for the existence of an "_ex_unwind" symbol via
dlsym() under some conditions when a thread exits, as part of an old
contract with a particular C++ standard library.  If another thread
exits at the same time that rustc is attempting to load a plugin, we can
hit this race and report an error that does not belong to us.
---
 src/librustc_metadata/dynamic_lib.rs | 31 ++++++++++++++++++++++------
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/src/librustc_metadata/dynamic_lib.rs b/src/librustc_metadata/dynamic_lib.rs
index ce19240a009d0..cd9f5f26003f3 100644
--- a/src/librustc_metadata/dynamic_lib.rs
+++ b/src/librustc_metadata/dynamic_lib.rs
@@ -63,9 +63,9 @@ mod dl {
         })
     }
 
-    fn check_for_errors_in<T, F>(f: F) -> Result<T, String>
+    fn check_for_errors_in<T, F>(f: F) -> Result<*mut T, String>
     where
-        F: FnOnce() -> T,
+        F: FnOnce() -> *mut T,
     {
         use std::sync::{Mutex, Once};
         static INIT: Once = Once::new();
@@ -77,16 +77,35 @@ mod dl {
             // dlerror isn't thread safe, so we need to lock around this entire
             // sequence
             let _guard = (*LOCK).lock();
+
+            // dlerror reports the most recent failure that occured during a
+            // dynamic linking operation and then clears that error; we call
+            // once in advance of our operation in an attempt to discard any
+            // stale prior error report that may exist:
             let _old_error = libc::dlerror();
 
             let result = f();
 
-            let last_error = libc::dlerror() as *const _;
-            if ptr::null() == last_error {
+            // We should only check dlerror() in the event that the operation
+            // fails, which we determine by checking for a NULL return.  This
+            // covers at least dlopen() and dlsym().
+            //
+            // While we are able to exclude other callers within this library,
+            // we are not able to exclude external callers such as those in the
+            // system libraries.  If dynamic linking activity is induced in
+            // another thread, it may destroy our dlerror() report or it may
+            // inject one that does not apply to us -- this error report must be
+            // treated as advisory.
+            if ptr::null() != result {
                 Ok(result)
             } else {
-                let s = CStr::from_ptr(last_error).to_bytes();
-                Err(str::from_utf8(s).unwrap().to_owned())
+                let last_error = libc::dlerror() as *const _;
+                if ptr::null() == last_error {
+                    Err("unknown dl error".to_string())
+                } else {
+                    let s = CStr::from_ptr(last_error).to_bytes();
+                    Err(str::from_utf8(s).unwrap().to_owned())
+                }
             }
         }
     }