Don't setenv in multi-threaded code on glibc
rachelbythebay.com
rachelbythebay.com
A quick look at the std::env::set_env docs [0] tells us that Rust is aware the underlying implementation are inherently thread-unsafe, and looking at its implementation [1] tells us that rust uses a global lock for all access to the environment.
So at least, it will avoid segfaulting your programs.
Good job rust :D
EDIT: However, I guess if you fork() (which is not something you can do with the standard rust library AFAIK, so requires unsafe), you may get the first problem of having a deadlock.
[0]: https://doc.rust-lang.org/std/env/fn.set_var.html
[1]: https://github.com/rust-lang/rust/blob/51d29343c04a27570a8ff...
https://github.com/rust-lang/rust/blob/0c85f2a1bdb01666e74cd...
Command::spawn() calls fork(). And you can schedule things to run on the child with CommandExt::before_exec().
EDIT: To be stabilized in 1.15 coming in two days.
Oh well, at least, it shouldn't be possible to invoke UB. Hopefully :D
Rust could hand-roll it by calling clone() directly, but the code is a bit tricky.
That's implying there are no setenv() calls in the C code that the Rust code calls.
Would I want my init system written in Java or C#? Hell no. Would I want it written in Rust? Yes please maybe it will crash less and have fewer CVEs.
I have a belief that every time a C developer switches to Rust, users benefit (unless developer goes crazy from Rust's cognitive load, but this is a separate issue :) )
Side note: Who even started the whole "x is a system programming language" ? Who said you can't write "system software"[0] in just about any language that can (directly or indirectly) do "lower level" things. For example the MS Singularity OS has a virtual machine and all the drivers and everything is high level code.
>Would I want my init system written in Java or C#? Hell no. Would I want it written in Rust? Yes please maybe it will crash less and have fewer CVEs.
Contrary to popular belief init systems are simple and can be written in any language. Not to mention that when people talk about inits they don't think about the actual initialization part (mounting, setting network, etc) but about the starting of programs part. Those two things are similar from a far but are really not so much.
>I have a belief that every time a C developer switches to Rust, users benefit (unless developer goes crazy from Rust's cognitive load, but this is a separate issue :) )
I have a belief that people should make up their own mind and do whatever they want. This topic is about C and only about C. All other talk is "noise".
Note that i actually like programming in C as i like optimized programs, so i may be biased. (although not nearly as biased as the rust crowd)
[0] https://en.wikipedia.org/wiki/System_programming_language
Why do you get to decide? There are a lot of people in this thread talking about similar issues in other languages (Python, Rust, etc) and other functions (like fopen() and exit()). I think those discussions are very on-topic.
Why isn't this topic about "issues between threads and setenv in any language that uses glibc"?
Why isn't this topic about "issues between threads and other functions in any language that uses glibc"?
If you don't like a particular conversation in this topic, you can collapse it and move on.
One other comment got it right, it is annoying to have multiple rust comments in every thread about C.
But you are right, best way to deal with these kinds of... i don't want to be rude, is ignoring.
PS Thank you for addressing the rest of the comment, not just the part you made personal.
But that is not the point...
In this case, for example, the underlying issue (as usual) isn't "C language BAD!" it's that the function call isn't thread-safe. Which can happen in any language, including Rust.
You can have a C that doesn't use glibc and doesn't have this problem. The question here is whether Rust, which in using the system libc will often use glibc on linux systems, has a way of working around this problem elegantly.
Unlike getenv, GetEnvironmentVariable copies a variable's value into a caller provided buffer, addressing the use-after-free problem inherent in getenv's interface.
Wouldn't it be nice if glibc got something like a getenv2 with a similar interface? Why should people have to trip over weird corner cases in POSIX over and over and over again? Why are we afraid to add new APIs that make some damn sense?
As far as I can tell glibc's implementation is probably what the people who designed getenv in the first place had in mind, just return a pointer to a global buffer. At best FreeBSD managed to create a workaround for modern environments.
There are unfortunately many such oddities in the standard libc APIs, remnants from an other time. Let us remember that "gets" was standardized as part of the C standard at some point, a function that's by design literally impossible to use safely.
It _is_ possible in theory to use gets safely, as long as your standard input is trusted. For instance, if a process forks and connects the standard input in the child to a pipe from the parent, which always writes a fixed amount of data to the pipe, you can use gets() without risk of overflow.
(This is a really contrived scenario, but I don't know of any simpler one where using gets() is safe.)
Only if we define "trusted" to include "known to be bug free" in e.g. it's truncation or bounding of output over the pipe to the child. I argue that, in theory, this is impossible to know, and thus that it this level of trust is impossible, and thus that this is not an example of a potential safe use of gets.
Even a mathematical proof of safety, after all, could contain errors - or could prove the wrong thing - or could apply to the code as written and not the code as messed with by your optimizer - or another thread - or an injected dll - or ...
> For instance, if a process forks and connects the standard input in the child to a pipe from the parent, which always writes a fixed amount of data to the pipe, you can use gets() without risk of overflow.
This is also insufficient - one must also prevent nonstandard invocations of the child process. Even if your normal parent process gives the child input that is 100% safe, that's no guarantee that an attacker won't launch your child process in an unusual manner. If the child process is suid, for example, this would be a potential avenue for privilege escalation.
"In the security engineering subspecialty of computer science, a trusted system is a system that is relied upon to a specified extent to enforce a specified security policy. As such, a trusted system is one whose failure may break a specified security policy." -- https://en.wikipedia.org/wiki/Trusted_system
That's the definition of "trusted" I'm using.
> This is also insufficient - one must also prevent nonstandard invocations of the child process.
I'm thinking of pure fork(), not fork+exec. That is, the child process is the same executable image, so the only way to invoke the child process in a nonstandard way would be through a debugger. And that is why the child process can trust the parent process in my example: they're the same process until the fork().
(As I said, it's a really contrived scenario. In more realistic scenarios, gets() is unsafe.)
Which thankfully was removed by recent standard revisions of C and C++, meaning a compliant compiler isn't required to provide it any longer.
How does FreeBSD's libc know when to free that snapshot? (Since there is no "free the env snapshot you gave me" call.) If thread B calls getenv, how does that not invalidate the snapshot that thread A has?
Other comments seem to imply this isn't what FreeBSD is doing however.
const char* foo = getenv("FOO");
const char* bar = getenv("BAR");
POSIX like any other paper standard implies write once debug everywhere.
The problem is the interface is naturally unsafe. Much like e.g. strcpy, the interface means it's nigh impossible to use it safely.
Hence why in parallel with better systems languages, we really need tagged architectures for pointers, like what SPARC V8, Intel MPX, Cherry offer, to become widespread, for better security.
I was replying about the whole UNIX API surface, regarding functions like strcpy.
You need exotic features to guarantee that the memory doesn't get corrupted.
I guess the situation might have improved.
It's access to the environ global that's inherently unsafe. setenv and a modified getenv can be accommodated just fine.
Lets say GNU/Linux does that, what you would do for other UNIX flavours, #ifdef soup?
The goal of standards is exactly to minimize that.
The only way we move forward is by trying new things, and we can't possibly get everyone to try a new thing all at once. If the thing ends up being useful, we standardize it.
Speak for yourself. I implemented a Standard C library which guarded the environment, and various other global variables that the language requires that the library have, with mutexes. I did this in the early 1990s.
I am far from alone, as far as C library implementors go. Your "we" is actually a small number of people.
The symptoms were intermittent segfaults in some high level scientific python code running on a large cluster in AWS. Given we had no custom C library extensions, this should be impossible, but there they were; the jobs would fail randomly for one in every several thousand runs, if memory serves.
We eventually tracked this to a call to getenv() in the OpenBLAS thread pool per-thread initialization code. This seems innocuous enough by itself, but couple it with some other unrelated python library which happened to call setenv() and you have a nice race condition accessing the internal glibc data structures. Ultimately this ended up with a segfault due to dereferencing some already freed memory.
Ah, good times. Here's the root cause analysis if anyone wants the gory details :-) https://github.com/xianyi/OpenBLAS/issues/716#issuecomment-1...
It goes like this:
1. Thread A calls FILE f = fopen(); if (f == NULL) {error}
2. Thread B calls exit(). exit() flushes and closes all open FILE (it does so it an internal atexit() handler)
3. Thread A determined that the file opened just fine, f != NULL. But Thread B has called fclose() on it, so any use in thread A is a use-after-free if Thread B is a tad slow in actually exiting the process
Do you have a glibc bug number?
Really, programs shouldn't expect _anything_ to happen on exit. After all, programs can die at any time for any reason anyway. Personally, I'd do away with both atexit and static destructors. Your program needs to be able to handle the power cord being yanked, so it needs to be able to handle sudden death no matter what.
glibc had a misfeature until 2.3 for _exit in MT programs, read the manpage ;)
_exit() will cause a similar race condition, but probably with less severe effects. _exit() closes all open file descriptors (not FILE*, like exit() does).
That means while exit() is executing, another thread could perform a file operation on a file descriptor that just was closed by the other thread running exit(). imo. glibc exit() implementation should halt/pause all the other threads before it goes on to mess with shared resources before it terminates the process.
Conceptually all that you described (plus some more, thing SysV IPC and MM) happens in the kernel (although the behaviour of streams is not specified, I can't think of any OS that does more than close FDs) after the OS has made it so every thread in the process can't go forward. I alluded to the misfeature in glibc, that's why you used to have to do raise(9) on linux to work around the oddity. Every other OS just did the right thing (though you sometimes had to build or link your program differently), ie all threads are 'stopped' before any of the things like closing FDs happens.
https://play.golang.org/p/E3glBkbAo3
(You'll need to run it locally for the full effect - things on play are limited to a single thread.)
It didn't crash.
I wasn't suprised though as Go doesn't use the C library and its authors are very careful about multithreaded code.
And likewise it would be subject to use-after-free races if it didn't call getenv but also didn't use a mutex to protect its accesses.
So the important bit is the mutex.
Say you call foo() in foo.c and foo() calls getenv() and setenv() with your mutex in the safeenv.go approach you described, all those calls from foo.c don't have the mutex around them that your foo.go uses. Worse yet it might not be your foo.c but part of libfoo in your OS.
They don't use getenv because it does a slow linear search, while Go can use a more efficient string->string map.
Also, if you want to pass an environment variable to a child process from a specific thread, how do you do that without interfering with other threads that might do the same (alter the same variable to pass it to a different child process)? Would that require an ugly mutex around invoking child processes?
As for how you do this sort of thing in general, fork/exec from a multithreaded POSIX program is a minefield and you're best off not doing it. But, if you insist, it looks like posix_spawn might help, or you can do your setenv in the child before the exec. setenv isn't async signal-safe, so strictly speaking you're not allowed to do this.
If this turns out actually to be a problem, introduce a second wrapper EXE that you spawn, supplying environment stuff and arguments for subchild EXE. Fork, exec the wrapper, have it do its thing, then have it exec the EXE you actually want (no fork).
I haven't done that for setting the environment, but I have for setting CLOEXEC on file descriptors when doing fork/exec as part of a (large, gnarly, zillions of FDs, tons of threads, closed source) program.
It doesn't have to be that complicated. After the fork, in the child, you can construct your own local environment array, and pass it to execve or one of the other exec functions that takes an environ argument. This is perfectly safe behavior in a multi-threaded program.
Never having needed to do exactly this myself in practice, I'd quite forgotten about the ...e functions.
You build up a the environment in a local variable, fork(), then have the child alter its environment based on the said local variable before it goes on to do useful stuff.
If you're just executing another program, do the same, but call execle() which takes the environment as an argument.
If only we had some sort of shared-nothing concurrency, like a syscall that forks your process and uses copy-on-write to make it efficient. But that's just crazy talk.
I suppose there is system() on non-POSIX, but then you don't have a portable shell anyway nor do you have portable environment variables. You're off in 100% non-portable territory at that point, so no point having it in the standards.
1. thread1 calls getenv() and gets a pointer to a section of environ A
2. thread2 calls setenv() & acquires lock & allocs environ B & frees environ A & releases lock
3. thread1 tries to look at the pointer in environ A, and crashes because environ A has been freed
Now, if you never call getenv(), and just call setenv() from multiple threads, that's safe, because setenv() itself is thread-safe. It's the combination that kills you.
The original os.environ is created somewhere else entirely. This file just sets it to an empty dict if it isn't already set[3].
os.environ is actually set in native code, here [4]. It is created by the createenviron function[5]. Now what you should note is that this is actually created as a python dict, and the native environment is copied into it. So a python program usually never touches the native environment after initialization. This is fine as long as only python functions are used to start child processes as these converts the environment into a native environment block and gives it as the envp pointer to the exec/spawn functions.
[1]: https://github.com/python/cpython/blob/a65137dc41fb52bdb1ca2...
[2]: https://github.com/python/cpython/blob/a65137dc41fb52bdb1ca2...
[3]: https://github.com/python/cpython/blob/a65137dc41fb52bdb1ca2...
[4]: https://github.com/python/cpython/blob/b9e40ed1bcce127893e40...
[5]: https://github.com/python/cpython/blob/b9e40ed1bcce127893e40...
no segfault detected