Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Mar 26, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 11 additions & 16 deletions src/libstd/sys/unix/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use prelude::v1::*;
use alloc::boxed::FnBox;
use cmp;
#[cfg(not(any(target_env = "newlib", target_os = "solaris")))]
use ffi::CString;
use ffi::CStr;
use io;
use libc;
use mem;
Expand Down Expand Up @@ -84,48 +84,43 @@ impl Thread {
#[cfg(any(target_os = "linux",
target_os = "android",
target_os = "emscripten"))]
pub fn set_name(name: &str) {
pub fn set_name(name: &CStr) {
const PR_SET_NAME: libc::c_int = 15;
let cname = CString::new(name).unwrap_or_else(|_| {
panic!("thread name may not contain interior null bytes")
});
// pthread wrapper only appeared in glibc 2.12, so we use syscall
// directly.
unsafe {
libc::prctl(PR_SET_NAME, cname.as_ptr() as libc::c_ulong, 0, 0, 0);
libc::prctl(PR_SET_NAME, name.as_ptr() as libc::c_ulong, 0, 0, 0);
}
}

#[cfg(any(target_os = "freebsd",
target_os = "dragonfly",
target_os = "bitrig",
target_os = "openbsd"))]
pub fn set_name(name: &str) {
let cname = CString::new(name).unwrap();
pub fn set_name(name: &CStr) {
unsafe {
libc::pthread_set_name_np(libc::pthread_self(), cname.as_ptr());
libc::pthread_set_name_np(libc::pthread_self(), name.as_ptr());
}
}

#[cfg(any(target_os = "macos", target_os = "ios"))]
pub fn set_name(name: &str) {
let cname = CString::new(name).unwrap();
pub fn set_name(name: &CStr) {
unsafe {
libc::pthread_setname_np(cname.as_ptr());
libc::pthread_setname_np(name.as_ptr());
}
}

#[cfg(target_os = "netbsd")]
pub fn set_name(name: &str) {
pub fn set_name(name: &CStr) {
use ffi::CString;
let cname = CString::new(&b"%s"[..]).unwrap();
let carg = CString::new(name).unwrap();
unsafe {
libc::pthread_setname_np(libc::pthread_self(), cname.as_ptr(),
carg.as_ptr() as *mut libc::c_void);
name.as_ptr() as *mut libc::c_void);
}
}
#[cfg(any(target_env = "newlib", target_os = "solaris"))]
pub fn set_name(_name: &str) {
pub fn set_name(_name: &CStr) {
// Newlib and Illumos has no way to set a thread name.
}

Expand Down
2 changes: 1 addition & 1 deletion src/libstd/sys/windows/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ impl Thread {
}
}

pub fn set_name(_name: &str) {
pub fn set_name(_name: &CStr) {
// Windows threads are nameless
// The names in MSVC debugger are obtained using a "magic" exception,
// which requires a use of MS C++ extensions.
Expand Down
21 changes: 18 additions & 3 deletions src/libstd/thread/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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,
}
Expand All @@ -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(),
})
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -622,6 +631,12 @@ mod tests {
}).unwrap().join().unwrap();
}

#[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();
Expand Down