8 ms·

But that won't actually fix the underlying problem, namely that getenv and setenv (or unsetenv, probably) cannot safely be called from different threads. It se

• pshc 2y ago
  The underlying problem is that setenv is mutable global state and should never have existed
  • Joker_vD 2y ago
    The process's current directory is mutable global state as well, and yet chdir(2) is thread-safe.
    • plorkyeran 2y ago
      chdir is thread-safe, but interacting with the current directory in any context other than parsing command-line arguments is still nearly always a mistake. Everything past a program's entry point should be working exclusively in absolute paths.
      • fanf2 2y ago
        Yeah if you chdir() in a multithreaded program, all cwd-relative file accesses in other threads are fucked. As well as absolute paths, it’s ok to work with descriptor-relative paths using openat() and friends.
    • account42 2y ago
      It's threadsafe in the memory sense. It's not threadsafe in the having an idea what files you are accessing sense.
      • Joker_vD 2y ago
        The latter is always true even when you don't use chdir(2) and/or always use absolute file paths since, you know, there are other processes that can re-arrange the file system whatsoever way the like. The file system is one example of the unavoidable global mutable shared state (another example is network) which one simply has to deal with.
        • account42 2y ago
          If files end up in a different directory because the user rearranged the filesystem under your nose, that's on the user. Most applications would deal with that by telling the user not to do that. 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.
    • fch42 2y ago
      The current working directory is kernel state. getcwd() is a system call. This doesn't compare.
    • kazinator 2y ago
      chdir is only thread safe to the extent that corruption won't occur. 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.
  • josefx 2y ago
    Welcome to the C standard library, the application of mutable global state to literally everything in it has to be the most consistent and predictable feature of the language standard.
    • Sharlin 2y ago
      I mean, I'm sure it was an okay solution on PDP-11.
    • liontwist 2y ago
      I used to think this was bad too. But when C was designed an entire single threaded program was considered the unit of encapsulation for functionality. Now it’s mostly libraries. 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.
  • bandrami 2y ago
    I mean, it's only threadsafe in the sense that opening a file in cwd without being able to actually know what cwd is is "safe"
• ModernMech 2y ago
  It's the same problem with global vars, but at a machine scope. The real solution here would be for the OS to have a better interface to read and write env vars, more like a file where you have to get rw permission (whether that's implemented as a mutex or what).
  • eqvinox 2y ago
    This is neither an OS nor a machine scope problem. The environment is provided by the OS at startup. What the process does with it from there on is its own concern.
    • ModernMech 2y ago
      > The environment is provided by the OS at startup. 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.
      • Joker_vD 2y ago
        Neither getenv, setenv nor realloc are system calls, they all are functions from C stdandard library, some parts of which for historical reasons are required to be almost impossible to use safely/reliably.
      • eqvinox 2y ago
        > getenv, setenv, and realloc, all system calls 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.) ed.: https://lwn.net/Articles/631631/ https://lwn.net/Articles/631631/
• eqvinox 2y ago
  I have a different perspective: the underlying problem is calling setenv(). As far as I'm concerned, the environment is a read-only input parameter set on process creation like argv. It's not a mechanism for exchanging information within a process, as used here with SSL_CERT_FILE. 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; Which is, of course, best described as bullshit.
  • diroussel 2y ago
    Indeed, environment variables should be used to configure child processes, not to configure the current process, for non-shell programs, IMHO. Note that Java, and the JVM, doesn't allow changing environment variables. It was the right choice, even if painful at times.
    • jamesfinlayson 2y ago
      Sure is painful (mostly when writing tests where the environment variables aren't abstracted in some way). But I think it was actually possible to hack around up until Java 17.
      • xxs 2y ago
        if you really wish - you can change the bootstrap path and allow changing env() for whatever reason you want to (likely via copy on write). If you don't wish to do that feel free to spawn a child process with whatever env you desire, then redirect/join sys in/our/err (0/1/2) 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)
        • jamesfinlayson 2y ago
• debugnik 2y ago
  No amount of locking can make the getenv API thread-safe, because it returns a pointer which gets invalidated by setenv, but lacks a way to release ownership over it and unblock setenv safely (or to free a returned copy). So setenv's existence makes getenv inherently unsafe unless you can ensure the entire application is at a safe point to use them.
  • josefx 2y ago
    C could provide functions to lock/unlock a mutex and require that any attempt to access the environment has to be done holding the mutex. This would still leave the correctness in the hands of the user, but at least it would provide a standard API to secure the environment in a multi threaded application that library and application developers could adopt.
    • fch42 2y ago
      That is basically "what it means" if an interface is non-MT: you can call this no-problem if you know you're singlethreaded, and if you're not, find your own way to serialize (meaning: have your own locking prinitive you acquire/release where you make calls to these functions). 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.
      • josefx 2y ago
        > because the nifty "frameworks" Malicious software exists, does that mean we should remove all threading primitives from the standard?
        • fch42 2y ago
          Obviously not, but _threading_ primitives are not the subject of this post at all. Declared-as Non-threadsafe interfaces are. And of course one (as is happening here) one can argue whether all "system runtimes" shall be threadsafe. Right now though, they are not, and agreed/sanctioned standards don't require them to be. Again (also as happening here) opinions may differ whether changes-to-make-threadsafe would be bugfixes, enhancements, or (require) new interfaces. I have expressed my views on this. Happy to agree to disagree, though.
• db48x 2y ago
  But it would force Rust programs to add their own synchronization mechanism around them. As long as no two threads can call getenv/setenv at the same time then it’s fine.
  • kibwen 2y ago
    The problem isn't something that Rust can solve. 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.
    • db48x 2y ago
      Ah, that’s a detail that I either forgot or did not know. Thank 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).
• Ferret7446 2y ago
  Is that a problem? I feel like calling getenv and setenv from different threads is a design antipattern anyway. Any environment setting and loading should happen in the one and only main thread right after process init.
• goeiedaggoeie 2y ago
  setenv and getenv have never been thread safe, why the concern with it now?
  • kibwen 2y ago
    The concern now is that, unlike when Posix was set in stone, threads exist.
    • loeg 2y ago
      The p in pthreads stands for Posix. I.e., uh, Posix is neither set in stone, nor entirely predates threads.
      • kibwen 2y ago
        And when can we expect the version of Posix that fixes setenv to be MT-safe?
        • loeg 2y ago
          You've shifted the goalposts from the comment I responded to.
          • kibwen 2y ago
            Not at all, because if Posix wasn't set in stone, then surely it would take somewhat less than three-ish decades to fix this problem.
      • pjmlp 2y ago
        I am old enough to remeber when UNIX only had processes, and several thread designs were being discussed until eventually pthreads one design won. POSIX predates adoption of threads in the UNIX world.
        • loeg 2y ago
• belter 2y ago
  > It seems like the only reliable way to fix this is to change these functions so that they exclusively acquire a mutex. A mutex can ensure thread safety but risks deadlocks if not used carefully and will hurt performance...
  • hamandcheese 2y ago
    Agree about performance, but wouldn't there need to be >1 mutex to risk a deadlock?
    • pixelesque 2y ago
      If it's not a "recursive mutex" (where you can call lock within the same thread on the same mutex more than once consecutively and it handled that), it's possible to lock on itself again (say in code which is recursive)...
    • loeg 2y ago
      Imagine you get a signal during getenv itself with the mutex held. Then your signal handler calls getenv. (On the other hand -- getenv is not marked async-signal-safe, so this use is already illegal.)
  • fch42 2y ago
    The problem (with get/set/putenv as they are) was isn't the non-use of a mutex. It's the "meaning" of the pointer returned to by getenv(). It returns a char*. Nevermind the persistance of that value - you can work around that by deliberately leaking memory - but it's writeable. Whether it's a good idea to do so ... well. But simply locking "inside" these funcs doesn't solve all the / your issues.
• liontwist 2y ago
  Please no. 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.
  • kazinator 2y ago
    That will break if any code which is not aware of the mutex calls getenv, even for a variable not related to the communication.
    • liontwist 2y ago
      Of course. It’s a bad idea. If you modify it the environemnt becomes a global var.
• loeg 2y ago
  Is that the underlying problem, or is the underlying problem that libraries are using thread-unsafe setenv in threaded contexts when they could just do something else?
• thayne 2y ago
  In particular, it doesn't help if you call a c function that indirectly modifies the environment with FFI.
• kazinator 2y ago
  The mutex would have to be held by the caller until it no longer needs the string returned from the environment, or makes a copy: 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 Other solutions: 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. 2. Provide environment lookup into a buffer: getenvbuf("HOME", mybuf, sizeof mybuf); // returns some value that helps to resize the buffer All functions that retain pointers out of the classic getenv remain unsafe. 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.