It seems like the only reliable way to fix this is to change these functions so that they exclusively acquire a mutex.
And remember that the exec* family of calls has a version with an envp argument, which is what should be used if a child process is to be started with a different environment — build a completely new structure, don't touch the existing one. Same for posix_spawn.
And, lastly, compatibility with ancient systems strikes again: the environment is also accessible through this:
extern char **environ;
Note that Java, and the JVM, doesn't allow changing environment variables. It was the right choice, even if painful at times.
I am fairly certain that somewhere inside the polyhedron that satisfies those constraints, is a large subset that could be statically analyzed and proven sound. But I'm less certain if Rust could express it cleanly.
struct BeforeEnvFreeze(());
struct AfterEnvFreeze(());
impl BeforeEnvFreeze {
pub fn new() -> Self { /* singleton check using a static AtomicBool or something */ Self(()) }
pub fn freeze(self) -> AfterEnvFreeze { AfterEnvFreeze(()) }
pub fn set_env(&self, ...) { ... }
}
impl AfterEnvFreeze {
pub fn spawn_thread(&self, ...) { ... }
}
fn main() {
let a = BeforeEnvFreeze::new();
a.set_env(...);
a.set_env(...);
//b.spawn_thread(...); // not available
let b = a.freeze(); // consumes `a`
b.spawn_thread(...);
//a.set_env(...); // not available
}
• Banning access to the relevant bits of Rust's stdlib, libc, etc. as a means of escaping this "safe" abstraction
• Conning your lead developer into accepting your handwave
• Setting up the appropriate VCS alerts so you have a chance to NAK "helpful" "utility" pull requests that undermine your "protections"
And of course, this all remains a hackaround for POSIX design flaws - your engineering time might be better spent ensuring or enforcing your libc is "fixed" via intentional memory leaks per e.g. https://github.com/bminor/glibc/commit/7a61e7f557a97ab597d6f... , which may ≈fix more than your Rust programs.
I'd go further and say env should always be read-only and libraries should never even read env vars.
I mean, based on this issue I would say the only safe time is "at the start of the program, before any new threads may have been created".
But again, as others have said, there's no good reason I'm aware of to set environment variables in your own process, and when you spawn a new process you can give it its own environment with any changes you want.
When using C++ I wanted programs to have a function that was called before main() and set up things that got sealed afterwards, like parsing command-line-arguments, the environment variables, loading runtime libraries, and maybe look at the local directory, but I'm not sure if it'll be a useful and meaningful distinction unless you restructure way too many things.
I remember that on the Fuchsia kernel programs needed to drop capabilities at some point, but the shift needed might be a hard sell given things already "work fine".
Not sure why would it be considered painful. Imo, use of setenv to modify your own variable, the definition of setenv is thread unsafe. So unless running a single threaded application it'd never make sense to call it.
Java does support running child processes with a designated env space (ProcessBuilder.environment is a modifiable map, copied from the current process), so inability to modify its own doesn't matter.
Personally I have never needed to change env variables. I consider them the same as the command line parameters.
Another reason why Java isn't the greatest language to create CLI tools with.
Linux and macOS both support per-thread working directory, although sadly through incompatible APIs.
Also, AFAIK, the Linux API can't restore the link between the process CWD and thread CWD once broken – you can change your thread's CWD back to the process CWD, but that thread won't pick up any future changes to the process CWD. By contrast, macOS has an API call to restore that link.
But I think it was actually possible to hack around up until Java 17.
Those are trivial things in around 100 lines of code and have been available since System.getenv() got back (it used to be deprecated and non-functional prior Java 1.5 or 2004)
Environmental variables are not a replacement for your config. It’s not a place to store your variables.
Even if the env var API is fully concurrent, it is not convention to write code that expects an env var to change. There isn’t even a mechanism for it. You’d have to write something to poll for changes and that should feel wrong.
The most common use I see for this is people setting an env in the current process before forking off a separate process; presumably because they don't realize that you can pass a new environment to new processes.
I wonder what bugs you'd find if you injected a library to override setenv() with a crash or error message into various programs. Might be a way to track down these kind of random irreproducable bugs.
I'd be happy to have you copy the immutable read-only environment vector of strings into your space and then treat that as the source of such things.
I think it would be interesting to build all the packages with a stdlib that dumps core on any call to setenv() or unsetenv(). That would give one an idea of the scope of the problem.
With good reason. Files are surprisingly hard: https://danluu.com/deconstruct-files/
Note the standard:
https://pubs.opengroup.org/onlinepubs/009604499/functions/se...
> The setenv() function need not be reentrant. A function that is not required to be reentrant is not required to be thread-safe.
With the increased use of PIE, thunks for both security and due to ARM + the difference between glibc and musl, plus busybox and you have a huge mess.
I would encourage you to play around with ghidra, just to see what return oriented programming and ARM limits does.
Compilers have been good at hiding those changes from us, but the non-reentrant nature will cause you issues even without threads.
Hint, these thunks can get inserted in the MI lowering stage or in the linker.
But setenv() is owned by posix, with only getenv() being differed to cppr.
Perhaps someone could submit a proposal on how to make it reentrant to the Open Group. But it wasn't really intended for maintaining mutable state so it may be a hard sell.
Agreed, 150%. My comment had more to do with rejecting files than it did with embracing environ as a suitable alternative.
SQLite is likely the most trouble-free option at the moment.
With that being said, it would be nice to see Android's sys/system_properties.h ported to GNU/Linux proper and, from there, other Unixen.
> I would encourage you to play around with ghidra, just to see what return oriented programming and ARM limits does.
Having worked professionally in reverse engineering at DoD, I can assure you that this is something I'm intimately familiar with.
Files and environ are bad.
For configuration files, the write-fsync-move strategy works fine. Generally you don't need fsync, since most people don't use the file system settings that allow data writes to be reordered with the metadata rename.
Modifying _your own_ environment _at runtime_ is not. The corresponding functions - setenv/getenv - and state - envp/environ - have in the UNIX standards "always" (since threads exist, really) been marked non-MT. "way back when" people were happy to accept that stated restrictions on use don't make bugs. Today, general sense of overentitlement makes (some) people say "but since whatever-trickery can remove this restriction... you're wrong and I'm entitled to my bugfix". I agree the damage is done, though.
This holds for a lot of programs, but what if you're writing a shell?
Of course, this means you won't see any changes to env vars from libraries you may use that call setenv(), but you also shouldn't need, or want, that in a shell.
I still think having a proper synchronous thread safe setenv()/getenv() in libc is the better choice.
"The getenv function returns a pointer to a string associated with the matched list member. The string pointed to shall not be modified by the program, but can be overwritten by a subsequent call to the getenv function."
In a single threaded virtual machine, you can immediately duplicate the string returned by getenv and stop using it, right there.
Under threads, getenv is not required to be safe.
I think that with some care, it may be; an environment implementation could guarantee that a non-mutating operation like getenv doesn't invalidate any previously returned strings.
I think POSIX does that. It allows getenv to reallocate the environ array, but not the strings themselves:
"Applications can change the entire environment in a single operation by assigning the environ variable to point to an array of character pointers to the new environment strings. After assigning a new value to environ, applications should not rely on the new environment strings remaining part of the environment, as a call to getenv(), secure_getenv(), [XSI] [Option Start] putenv(), [Option End] setenv(), unsetenv(), or any function that is dependent on an environment variable may, on noticing that environ has changed, copy the environment strings to a new array and assign environ to point to it."
environ is documented together with the exec family of functions; that's where this is found.
So whereas there are things not to like about environ, it can be the basis for thread safety of getenv in an application that doesn't mutate the environment.
Mutating argv is actually quite popular, or at least it used to be.
It is fine because it is usually done during the initialization phase, before starting any other thread. setenv() can be used here too, though I prefer to avoid doing that in any case. I also prefer not to touch argv, but since that's how GNU getopt() works, I just go with it.
Once the program is running and has started its threads, I consider setenv() is a big no no. The Rust documentation agrees with me: "In multi-threaded programs on other operating systems, the only safe option is to not use set_var or remove_var at all.". Note: here, "other operating systems" means "not Windows".
¹ or technically whatever your ELF entry point is, _start in crt0 or your poison of choice.
> ¹ or technically whatever your ELF entry point is, _start in crt0 or your poison of choice.
Once you include the footnote, at least on linux/macos (not sure about Windows), you could take the same perspective with regards to envp and the auxiliary array. It's libc that decided to store a pointer to these before calling your `main`, not the abi. At the time of the ELF entry point these are all effectively stack local variables.
The API is ugly, and since it needs CAP_SYS_RESOURCE many programs can't use it... but systemd does: https://github.com/systemd/systemd/blob/2635b5dc4a96157c2575...
This shouldn't cause the kind of race conditions we are talking about here, since it isn't changing a single arg, it is changing the whole argv all at once. However, the fact that PR_SET_MM_ARG_START/PR_SET_MM_ARG_END are two separate prctl syscalls potentially introduces a different race condition. If Linux would only provide a prctl to set both at once, that would fix that. The reason it was done this way, is the API was originally designed for checkpoint-restore, in which case the process will be effectively suspended while these calls are made.
So setenv's existence makes getenv inherently unsafe unless you can ensure the entire application is at a safe point to use them.
Getenv() could keep several copies of the value around: one internal copy protected by a mutex, that it never returns, and one copy per thread that it stores in thread local storage. When you call getenv(), it locks the mutex, checks if the current thread's value exists, populates it from the internal copy if not, and returns it. It will also install a new setenv-specific signal handler on this thread and store info about this thread having a copy.
Setenv() will then take the same mutex as getenv(), check if the internal copy is different from the new value; if it is, it will modify the internal copy, modify the local thread's copy if that has one, and then signal each other thread in the process that has a copy in TLS. The setenv signal handler will modify the local copy that thread holds.
It's gonna be slow for a large multi-threaded program, but since setenv() used to corrupt memory for such programs, they probably don't care. And for single-threaded programs, or even for programs that don't access getenv()/setenv() on multiple threads, there should be no extra overhead other than the mutex and the bookkeeping.
The only issues that would remain are programs which send the pointer they get from getenv() to other threads without ensuring locking access, and programs which rely on modifying the pointer from getenv() directly as a way to set an env var, and expect this to be visible across threads. Those are just hopelessly broken and can't use the same API - but aren't more broken then they are today.
Of course, in addition to this complex work to make the old API (mostly) thread safe, it should also offer a new API that simply returns a copy every time, doesn't promise to show modifications to your copy when setenv() gets called (you need to call getenv() again), and puts the onus on you to free that copy explicitly.
Returning a copy isn't great (memory allocation!), the API should probably be something like:
int getenv(const char *varName, char *buf, size_t bufSize, size_t *varSize);
Doing it like this, you can easily implement the "return a malloced copy" if you want to, but it also gives you the option to avoid allocation entirely. This is important for e.g. embedded or real-time applications, or anything that just likes to avoid `malloc()/free()`.
Your particular solution doesn't work because people expect `getenv` to be async-signal-safe, which means you shouldn't be allocating memory.
Hmm ... doing an incref-like operation during `getenv` for a previously `setenv`ed variable that hasn't yet been accessed in this thread would be fine ... clear those refs during calls we know indicate knowledge refreshes ...
It's equally nasty. POSIX requires that the argument to `putenv()' not be copied, so it's not very different from assigning to `environ' directly.
"easy": protect the page containing environ and handle the mutation from the signal handler.
/s of course.
Btw, you can _also_ substitute libc's setenv/getenv/putenv with your own (locking) implementations, courtesy preload and all the funky features of ELF symbol resolution. Actually easy. But impossible if you link against static code using it (go ... away). Hmm. easy ? impossible ? damn this grey world. Gimme some color.
It looks useful.
And that's before you even get to the `extern char *environ` global.
One could "dream of" a func that tells libc "acquire/drop this mutex of mine around get/set/putenv calls" but that'd simply move the problem - because the nifty "frameworks" would do that (independently of each other, we're sovereign and entitled frameworks around here) and race each other's state nonetheless.
Malicious software exists, does that mean we should remove all threading primitives from the standard?
If your sensitive logs end up in the webserver root because one thread used chdir to temporarily change the working directory it's on the application writer.
Or to put it another way, the filesystem as a whole being shared mutable state does not make the current working directory being shared mutable state between threads any less of an issue.
As well as absolute paths, it’s ok to work with descriptor-relative paths using openat() and friends.
If one thread is using relative paths, and another is doing a chdir-based traversal (as using the nftw function, for instance), that first thread's accesses are messed up.
This is why POSIX now has various -at functions; the provide stable relative access.
The former allows you to design a coherent system. a lot of design questions which are annoying (“how do I access config data consistently, etc) become very clear.
It also makes C more productive. If global vars and static locals are unbanned, features like closures become less important.
stdenvlock(); // imaginary function added to ISO C or POSIX
char *home = getenv("HOME");
char *home_copy = strdup(home);
stdenvunlock(); // only here can we unlock
// home pointer is now indeterminate
1. Put the above sequence into a function, and don't expose the mutex. Thread-safe code must use:
char *home = dupenv("HOME"); // imaginary function; caller responsible for freeing.
getenvbuf("HOME", mybuf, sizeof mybuf); // returns some value that helps to resize the buffer
A mutex can be provided to those applications that want to manipulate the environ array directly, or use getenv and setenv, or any combinations of these.
The main problem is all the code out there using getenv.
If your program wants to use the environment as an out-of-band global var for cross thread communication, you can make your own mutex.
That's part of the design of the OS. How the OS implements this is primitive, and so it leaves it up to every language to handle. The blog mentions the issue is with getenv, setenv, and realloc, all system calls. To me, that sounds like bad OS design is causing issues downstream with languages, leaving it up to individual programmers to deal with the fallout.
None of these 3 functions is a system call. open(), mmap(), sbrk(), poll(), etc. are system calls. What you're referring to is C library API, which as Go has shown (both to its benefit and its detriment) is optional on almost all operating systems (a major exception being OpenBSD.)
If you really want to lose some sanity I would recommend reading the man page for getauxval(), and then look up how that works on the machine level when the process is started. Especially on some of the older architectures. (No liability accepted for any grey hair induced by this.)
A mutex can ensure thread safety but risks deadlocks if not used carefully and will hurt performance...
POSIX predates adoption of threads in the UNIX world.
The Rust stdlib is already using synchronization on the versions of these functions that are exposed from the Rust stdlib. That's why those functions were allowed to be marked as safe in the first place.
The problem is that people are calling C code from Rust (which already requires an unsafe annotation), and then that C code is doing silly thread-unsafe shenanigans for regrettable historical reasons.
It's beyond Rust's power to fix without cooperation from the underlying C code, which happens to be provided by the OS, which is just being compliant with Posix. Rust can only do so much when the platform itself is hell-bent on sabotaging you.
It certainly would be nice if the C library had fewer built–in footguns. And if we could write programs in other languages without ever depending on it (which wouldn’t but much use when you’re relying on a C library anyway, but it still would be nice).
Nice to see that the author of the library has a sensible take. Unfortunately the ecosystem does not: https://github.com/seanmonstar/reqwest/blob/master/Cargo.tom...
This also shows up in web frameworks where Vue has the v-html directive and react has dangerouslySetInnerHTML. Vue definitely has it better.
If DOM nodes during the next render differ from what react-dom expects (i.e. the DOM nodes from the previous render), then react-dom may throw a DOMException. Mutating innerHTML via a ref may violate React's invariants, and the library correctly throws an error when programmers, browser extensions, etc. mutate the DOM such that a node's parent unexpectedly changes.
There are workarounds[1] to mutate DOM nodes managed by React and avoid DOMExceptions, but I haven't worked on a codebase where anything like this was necessary.
[1] https://github.com/facebook/react/issues/11538#issuecomment-...
innerHTML is useful when there is a trusted HTML source, which is becoming more popular with stuff like HTMX and FastHTML.