Skip to content

Commit 605f28f

Browse files
committed
[rust] Bypass SB2 gated calls which deadlock. Contributes JB#50499
When rust's Command class does a spawn() call (at the rust level) it invokes either libc::spawnvp() or libc::fork()/exec(). This is problematic because cargo is heavily threaded and fork/exec in threads is known to be prone to deadlock due to futex sharing. (google it!) Whilst rust takes care to manage malloc, sb2 does not and calling sb2 between the fork and exec in a threaded application under SB2 will lead to deadlock. This patch introduces ENV triggered bypasses for libc calls made in Command.spawn(). This includes, dup2, chdir and close. This patch does not yet fix all hangs under sb2. (Eg the close() call is invoked during the Drop trait of the shared pipe IPC... there may be others hiding somewhere). Don't strip when cross-building libs as it corrupts them Signed-off-by: David Greaves <[email protected]>
1 parent 2a34515 commit 605f28f

File tree

2 files changed

+356
-1
lines changed

2 files changed

+356
-1
lines changed
Lines changed: 347 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,347 @@
1+
From 596ba5c8339b803b19734358e464fb25ca2786e6 Mon Sep 17 00:00:00 2001
2+
From: David Greaves <[email protected]>
3+
Date: Wed, 20 Jan 2021 09:40:34 +0000
4+
Subject: [PATCH] Provide ENV controls to bypass some sb2 calls between fork
5+
exec
6+
7+
In threaded systems there can be deadlocks caused by calling malloc
8+
after fork and before exec. Rust manages this but sb2 does not.
9+
10+
The following ENV variables are available:
11+
12+
SB2_RUST_EXECVP_SHIM
13+
14+
This executable is passed to the execvp call. The intended executable
15+
is passed in the argv[] list. Typically set to "/usr/bin/env --". Used
16+
wth SB2_RUST_USE_REAL_EXECVP which will not do path processing to exec
17+
this binary but the binary will then exec the command provided via sb2
18+
lookups.
19+
20+
SB2_RUST_USE_REAL_EXECVP
21+
When set will cause the libc execvp() to be called and not the sb2
22+
gate
23+
24+
SB2_RUST_USE_REAL_FN
25+
This ensures that no sb2 calls to gates for dup2/chdir etc are made
26+
between fork and exec
27+
28+
SB2_RUST_NO_SPAWNVP
29+
When set the rust libc::spawnvp() call (which is gated) is bypassed
30+
and the fallback fork/exec is used.
31+
32+
Note that some reworking of rust's handling of program/argv[0] was
33+
needed to support the SHIM functionality
34+
35+
Signed-off-by: David Greaves <[email protected]>
36+
---
37+
src/libstd/sys/unix/process/process_common.rs | 40 ++++-
38+
src/libstd/sys/unix/process/process_unix.rs | 138 ++++++++++++++++--
39+
2 files changed, 160 insertions(+), 18 deletions(-)
40+
41+
diff --git a/src/libstd/sys/unix/process/process_common.rs b/src/libstd/sys/unix/process/process_common.rs
42+
index 859da691ad2..ee5e776efac 100644
43+
--- a/src/libstd/sys/unix/process/process_common.rs
44+
+++ b/src/libstd/sys/unix/process/process_common.rs
45+
@@ -71,10 +71,16 @@ pub struct Command {
46+
// located. Whenever we add a key we update it in place if it's already
47+
// present, and whenever we remove a key we update the locations of all
48+
// other keys.
49+
- program: CString,
50+
+ pub(crate) program: CString,
51+
args: Vec<CString>,
52+
argv: Argv,
53+
env: CommandEnv,
54+
+ pub(crate) execvp: Option<ExecvpFn>,
55+
+ pub(crate) dup2: Option<Dup2Fn>,
56+
+ pub(crate) close: Option<CloseFn>,
57+
+ pub(crate) chdir: Option<ChdirFn>,
58+
+ pub(crate) setuid: Option<SetuidFn>,
59+
+ pub(crate) setgid: Option<SetgidFn>,
60+
61+
cwd: Option<CString>,
62+
uid: Option<uid_t>,
63+
@@ -86,6 +92,13 @@ pub struct Command {
64+
stderr: Option<Stdio>,
65+
}
66+
67+
+pub(crate) type ExecvpFn = fn(*const c_char, *const *const c_char)->c_int;
68+
+pub(crate) type Dup2Fn = fn(c_int, c_int)->c_int;
69+
+pub(crate) type CloseFn = fn(c_int)->c_int;
70+
+pub(crate) type ChdirFn = fn(*const c_char)->c_int;
71+
+pub(crate) type SetuidFn = fn(uid_t)->c_int;
72+
+pub(crate) type SetgidFn = fn(gid_t)->c_int;
73+
+
74+
// Create a new type for argv, so that we can make it `Send`
75+
struct Argv(Vec<*const c_char>);
76+
77+
@@ -130,15 +143,22 @@ impl Command {
78+
pub fn new(program: &OsStr) -> Command {
79+
let mut saw_nul = false;
80+
let program = os2c(program, &mut saw_nul);
81+
+ let arg0 = program.clone();
82+
Command {
83+
- argv: Argv(vec![program.as_ptr(), ptr::null()]),
84+
- args: vec![program.clone()],
85+
- program,
86+
+ argv: Argv(vec![arg0.as_ptr(), ptr::null()]),
87+
+ args: vec![arg0],
88+
+ program: program,
89+
env: Default::default(),
90+
+ execvp: None,
91+
+ dup2: None,
92+
+ close: None,
93+
+ chdir: None,
94+
+ setuid: None,
95+
+ setgid: None,
96+
cwd: None,
97+
uid: None,
98+
gid: None,
99+
- saw_nul,
100+
+ saw_nul: saw_nul,
101+
closures: Vec::new(),
102+
stdin: None,
103+
stdout: None,
104+
@@ -146,6 +166,16 @@ impl Command {
105+
}
106+
}
107+
108+
+ // This allows process_unix::{spawn, exec} to push program to the
109+
+ // start of /usr/bin/env's arg list
110+
+ pub fn insert_program(&mut self, arg: String) {
111+
+ let arg = OsString::from(arg);
112+
+ let arg = os2c(&arg, &mut self.saw_nul);
113+
+ self.program = arg.clone();
114+
+ self.argv.0.insert(0, arg.as_ptr());
115+
+ self.args.insert(0, arg);
116+
+ }
117+
+
118+
pub fn set_arg_0(&mut self, arg: &OsStr) {
119+
// Set a new arg0
120+
let arg = os2c(arg, &mut self.saw_nul);
121+
diff --git a/src/libstd/sys/unix/process/process_unix.rs b/src/libstd/sys/unix/process/process_unix.rs
122+
index f389c60615f..d5763b8aa1a 100644
123+
--- a/src/libstd/sys/unix/process/process_unix.rs
124+
+++ b/src/libstd/sys/unix/process/process_unix.rs
125+
@@ -5,7 +5,10 @@ use crate::sys;
126+
use crate::sys::cvt;
127+
use crate::sys::process::process_common::*;
128+
129+
-use libc::{c_int, gid_t, pid_t, uid_t};
130+
+use libc::{c_int, gid_t, pid_t, uid_t, dlsym, c_char};
131+
+use crate::intrinsics::transmute;
132+
+use crate::ffi::{OsString};
133+
+use sys::os::getenv;
134+
135+
////////////////////////////////////////////////////////////////////////////////
136+
// Command
137+
@@ -33,6 +36,70 @@ impl Command {
138+
139+
let (input, output) = sys::pipe::anon_pipe()?;
140+
141+
+ // If there is a RUST_EXEC_SHIM (could be "/usr/bin/env --")
142+
+ // then we're probably going to directly execvp it via dlsym
143+
+ // to avoid issues with threads and malloc post-fork and
144+
+ // pre-exec. That will then re-execvp but this time sb2 will
145+
+ // do magic. See also RUST_EXECVP_REAL
146+
+
147+
+ // We do this here and pass so do_exec() so any malloc's are
148+
+ // pre-fork()
149+
+
150+
+ // At this point self.program is the real program. argv[0] is
151+
+ // now a clone() of program.
152+
+
153+
+ let libc_h = unsafe { libc::dlopen("libc.so.6\0".as_ptr() as *const c_char,
154+
+ libc::RTLD_LAZY) };
155+
+
156+
+ match getenv(&OsString::from("SB2_RUST_EXECVP_SHIM"))? {
157+
+ Some(var) => { // handle "/usr/bin/env <arg> <arg>"
158+
+ let var = var.into_string().expect("Valid string"); // so we can .split()
159+
+ let words: Vec<&str> = var.as_str().split(" ").collect();
160+
+ for w in words.iter().rev() {
161+
+ self.insert_program(w.to_string());
162+
+ };
163+
+ // At this point self.program is the SHIM. argv[0] is
164+
+ // the SHIM and argv[>0] is the real program.
165+
+ },
166+
+ None => {} // Business as usual
167+
+ };
168+
+ match getenv(&OsString::from("SB2_RUST_USE_REAL_EXECVP"))? {
169+
+ Some(_var) => unsafe {
170+
+ let real_execvp_p = dlsym(libc_h,
171+
+ "execvp\0".as_ptr() as *const c_char) as *const ();
172+
+ self.execvp = Some(
173+
+ transmute::<*const (), ExecvpFn>(real_execvp_p) );
174+
+ },
175+
+ None => {}
176+
+ };
177+
+ match getenv(&OsString::from("SB2_RUST_USE_REAL_FN"))? {
178+
+ Some(_var) => unsafe {
179+
+ let real_dup2_p = dlsym(libc_h,
180+
+ "dup2\0".as_ptr() as *const c_char) as *const ();
181+
+ self.dup2 = Some(
182+
+ transmute::<*const (), Dup2Fn>(real_dup2_p) );
183+
+ let real_close_p = dlsym(libc_h,
184+
+ "close\0".as_ptr() as *const c_char) as *const ();
185+
+ self.close = Some(
186+
+ transmute::<*const (), CloseFn>(real_close_p) );
187+
+ let real_chdir_p = dlsym(libc_h,
188+
+ "chdir\0".as_ptr() as *const c_char) as *const ();
189+
+ self.chdir = Some(
190+
+ transmute::<*const (), ChdirFn>(real_chdir_p) );
191+
+ let real_setuid_p = dlsym(libc_h,
192+
+ "setuid\0".as_ptr() as *const c_char) as *const ();
193+
+ self.setuid = Some(
194+
+ transmute::<*const (), SetuidFn>(real_setuid_p) );
195+
+ let real_setgid_p = dlsym(libc_h,
196+
+ "setgid\0".as_ptr() as *const c_char) as *const ();
197+
+ self.setgid = Some(
198+
+ transmute::<*const (), SetgidFn>(real_setgid_p) );
199+
+ },
200+
+ None => {}
201+
+ };
202+
+ // We close before calling but that's OK as this is just a lookup handle
203+
+ unsafe { cvt(libc::dlclose(libc_h))? };
204+
+
205+
// Whatever happens after the fork is almost for sure going to touch or
206+
// look at the environment in one way or another (PATH in `execvp` or
207+
// accessing the `environ` pointer ourselves). Make sure no other thread
208+
@@ -45,11 +112,11 @@ impl Command {
209+
let _env_lock = sys::os::env_lock();
210+
cvt(libc::fork())?
211+
};
212+
-
213+
+
214+
let pid = unsafe {
215+
match result {
216+
0 => {
217+
- drop(input);
218+
+ self.unwrap_drop(input);
219+
let Err(err) = self.do_exec(theirs, envp.as_ref());
220+
let errno = err.raw_os_error().unwrap_or(libc::EINVAL) as u32;
221+
let bytes = [
222+
@@ -135,7 +202,37 @@ impl Command {
223+
Err(e) => e,
224+
}
225+
}
226+
-
227+
+ fn unwrap_drop(&mut self, fh: sys::unix::pipe::AnonPipe) {
228+
+ // drop() simply calls libc::close(fh.fd)
229+
+ match self.close {
230+
+ Some(real_close) => { (real_close)(fh.fd().raw()); },
231+
+ None => { drop(fh); }
232+
+ }
233+
+ }
234+
+ fn unwrap_dup2(&mut self, src: c_int, dst: c_int) -> c_int {
235+
+ match self.dup2 {
236+
+ Some(real_dup2) => { (real_dup2)(src, dst) },
237+
+ None => { unsafe { libc::dup2(src, dst) } }
238+
+ }
239+
+ }
240+
+ fn unwrap_chdir(&self, dir: *const c_char) -> c_int {
241+
+ match self.chdir {
242+
+ Some(real_chdir) => { (real_chdir)(dir) },
243+
+ None => { unsafe { libc::chdir(dir) } }
244+
+ }
245+
+ }
246+
+ fn unwrap_setuid(&self, uid: uid_t) -> c_int {
247+
+ match self.setuid {
248+
+ Some(real_setuid) => { (real_setuid)(uid) },
249+
+ None => { unsafe { libc::setuid(uid) } }
250+
+ }
251+
+ }
252+
+ fn unwrap_setgid(&self, gid: gid_t) -> c_int {
253+
+ match self.setgid {
254+
+ Some(real_setgid) => { (real_setgid)(gid) },
255+
+ None => { unsafe { libc::setgid(gid) } }
256+
+ }
257+
+ }
258+
// And at this point we've reached a special time in the life of the
259+
// child. The child must now be considered hamstrung and unable to
260+
// do anything other than syscalls really. Consider the following
261+
@@ -174,19 +271,19 @@ impl Command {
262+
use crate::sys::{self, cvt_r};
263+
264+
if let Some(fd) = stdio.stdin.fd() {
265+
- cvt_r(|| libc::dup2(fd, libc::STDIN_FILENO))?;
266+
+ cvt_r(|| self.unwrap_dup2(fd, libc::STDIN_FILENO))?;
267+
}
268+
if let Some(fd) = stdio.stdout.fd() {
269+
- cvt_r(|| libc::dup2(fd, libc::STDOUT_FILENO))?;
270+
+ cvt_r(|| self.unwrap_dup2(fd, libc::STDOUT_FILENO))?;
271+
}
272+
if let Some(fd) = stdio.stderr.fd() {
273+
- cvt_r(|| libc::dup2(fd, libc::STDERR_FILENO))?;
274+
+ cvt_r(|| self.unwrap_dup2(fd, libc::STDERR_FILENO))?;
275+
}
276+
277+
#[cfg(not(target_os = "l4re"))]
278+
{
279+
if let Some(u) = self.get_gid() {
280+
- cvt(libc::setgid(u as gid_t))?;
281+
+ cvt(self.unwrap_setgid(u as gid_t))?;
282+
}
283+
if let Some(u) = self.get_uid() {
284+
// When dropping privileges from root, the `setgroups` call
285+
@@ -199,11 +296,11 @@ impl Command {
286+
//FIXME: Redox kernel does not support setgroups yet
287+
#[cfg(not(target_os = "redox"))]
288+
let _ = libc::setgroups(0, ptr::null());
289+
- cvt(libc::setuid(u as uid_t))?;
290+
+ cvt(self.unwrap_setuid(u as uid_t))?;
291+
}
292+
}
293+
if let Some(ref cwd) = *self.get_cwd() {
294+
- cvt(libc::chdir(cwd.as_ptr()))?;
295+
+ cvt(self.unwrap_chdir(cwd.as_ptr()))?;
296+
}
297+
298+
// emscripten has no signal support.
299+
@@ -250,9 +347,17 @@ impl Command {
300+
_reset = Some(Reset(*sys::os::environ()));
301+
*sys::os::environ() = envp.as_ptr();
302+
}
303+
-
304+
- libc::execvp(self.get_program().as_ptr(), self.get_argv().as_ptr());
305+
- Err(io::Error::last_os_error())
306+
+ match self.execvp {
307+
+ Some(real_execvp) => {
308+
+ (real_execvp)(self.get_program().as_ptr(),
309+
+ self.get_argv().as_ptr())
310+
+ },
311+
+ None => {
312+
+ libc::execvp(self.get_program().as_ptr(),
313+
+ self.get_argv().as_ptr())
314+
+ }
315+
+ };
316+
+ Err(io::Error::last_os_error())
317+
}
318+
319+
#[cfg(not(any(
320+
@@ -265,6 +370,7 @@ impl Command {
321+
_: &ChildPipes,
322+
_: Option<&CStringArray>,
323+
) -> io::Result<Option<Process>> {
324+
+ eprintln!("process_unix:270: in null posix_spawn");
325+
Ok(None)
326+
}
327+
328+
@@ -283,10 +389,16 @@ impl Command {
329+
use crate::mem::MaybeUninit;
330+
use crate::sys;
331+
332+
+ let skip_spawnvp :bool = match getenv(&OsString::from("SB2_RUST_NO_SPAWNVP"))? {
333+
+ Some(_var) => true,
334+
+ None => false
335+
+ };
336+
+
337+
if self.get_gid().is_some()
338+
|| self.get_uid().is_some()
339+
|| self.env_saw_path()
340+
|| !self.get_closures().is_empty()
341+
+ || skip_spawnvp
342+
{
343+
return Ok(None);
344+
}
345+
--
346+
2.20.1
347+

rust.spec

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@ Patch2: 0002-Set-proper-llvm-targets.patch
6161
Patch3: 0003-Disable-statx-for-all-builds.-JB-50106.patch
6262
Patch4: 0004-Scratchbox2-needs-to-be-able-to-tell-rustc-the-defau.patch
6363
Patch5: 0005-Cargo-Force-the-target-when-building-for-CompileKind-Host.patch
64+
Patch6: 0006-Provide-ENV-controls-to-bypass-some-sb2-calls-betwee.patch
6465
# This is the real rustc spec - the stub one appears near the end.
6566
%ifarch %ix86
6667

@@ -143,6 +144,9 @@ Requires: /usr/bin/cc
143144
%global _find_debuginfo_opts -g
144145
%undefine _include_minidebuginfo
145146

147+
# Don't strip the rlibs as this fails for cross-built libraries
148+
%define __strip /bin/true
149+
146150
%global rustflags -Clink-arg=-Wl,-z,relro,-z,now
147151

148152
%description
@@ -238,6 +242,7 @@ test -f '%{local_rust_root}/bin/rustc'
238242
%patch3 -p1
239243
%patch4 -p1
240244
%patch5 -p1
245+
%patch6 -p1
241246

242247
sed -i.try-py3 -e '/try python2.7/i try python3 "$@"' ./configure
243248

@@ -303,7 +308,7 @@ FFLAGS=
303308
%endif
304309

305310

306-
# arm cc needs to find ld so ensure PATH points there
311+
# arm cc needs to find ld so ensure PATH points to /opt/cross/bin too
307312
PATH=/opt/cross/bin/:$PATH
308313

309314
###
@@ -392,6 +397,9 @@ find %{buildroot}%{_libdir} -maxdepth 1 -type f -name '*.so' \
392397
# The non-x86 .so files would be used by rustc if it had been built
393398
# for those targets
394399
rm %{buildroot}%{rustlibdir}/%{rust_arm_triple}/lib/*.so
400+
%if 0%{?build_aarch64}
401+
rm %{buildroot}%{rustlibdir}/%{rust_aarch64_triple}/lib/*.so
402+
%endif
395403

396404
# Remove installer artifacts (manifests, uninstall scripts, etc.)
397405
find %{buildroot}%{rustlibdir} -maxdepth 1 -type f -exec rm -v '{}' '+'

0 commit comments

Comments
 (0)