Fix unsound behaviour with null characters in thread names (issue #32475)

Previously, the thread name (&str) was converted to a CString in the
new thread, but outside unwind::try, causing a panic to continue into FFI.

This patch changes that behaviour, so that the panic instead happens
in the parent thread (where panic infrastructure is properly set up),
not the new thread.

This could potentially be a breaking change for architectures who don't
support thread names.

Signed-off-by: David Henningsson <diwic@ubuntu.com>

1 files changed, 18 insertions, 3 deletions

diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs
index f3139aaf98d..b3549dc1264 100644
--- a/src/libstd/thread/mod.rs
+++ b/src/libstd/thread/mod.rs
@@ -166,6 +166,8 @@ use any::Any;
 use cell::UnsafeCell;
 use fmt;
 use io;
+use str;
+use ffi::{CStr, CString};
 use sync::{Mutex, Condvar, Arc};
 use sys::thread as imp;
 use sys_common::thread_info;
@@ -267,7 +269,7 @@ impl Builder {
         let their_packet = my_packet.clone();
 
         let main = move || {
-            if let Some(name) = their_thread.name() {
+            if let Some(name) = their_thread.cname() {
                 imp::Thread::set_name(name);
             }
             unsafe {
@@ -450,7 +452,7 @@ pub fn park_timeout(dur: Duration) {
 
 /// The internal representation of a `Thread` handle
 struct Inner {
-    name: Option<String>,
+    name: Option<CString>,      // Guaranteed to be UTF-8
     lock: Mutex<bool>,          // true when there is a buffered unpark
     cvar: Condvar,
 }
@@ -465,9 +467,12 @@ pub struct Thread {
 impl Thread {
     // Used only internally to construct a thread object without spawning
     fn new(name: Option<String>) -> Thread {
+        let cname = name.map(|n| CString::new(n).unwrap_or_else(|_| {
+            panic!("thread name may not contain interior null bytes")
+        }));
         Thread {
             inner: Arc::new(Inner {
-                name: name,
+                name: cname,
                 lock: Mutex::new(false),
                 cvar: Condvar::new(),
             })
@@ -489,6 +494,10 @@ impl Thread {
     /// Gets the thread's name.
     #[stable(feature = "rust1", since = "1.0.0")]
     pub fn name(&self) -> Option<&str> {
+        self.cname().map(|s| unsafe { str::from_utf8_unchecked(s.to_bytes()) } )
+    }
+
+    fn cname(&self) -> Option<&CStr> {
         self.inner.name.as_ref().map(|s| &**s)
     }
 }
@@ -623,6 +632,12 @@ mod tests {
     }
 
     #[test]
+    #[should_panic]
+    fn test_invalid_named_thread() {
+        let _ = Builder::new().name("ada l\0velace".to_string()).spawn(|| {});
+    }
+
+    #[test]
     fn test_run_basic() {
         let (tx, rx) = channel();
         thread::spawn(move|| {