Function to hold lock on execution environment (for FFI)

libs
21

For what it's worth a musl maintainer has also stated that, and I think backwards compatibility is going to to prevent a cleaner solution.

1 Like
22

I'm afraid that such lock would be an incomplete solution, and more of an illusion of safety. It only protects Rust code, no the whole process, and only protects from a fraction of foreign code that is under Rust's control.

  • it doesn't protect Rust from C code that isn't running under the lock (e.g. if the FFI code spawns a thread that outlives the lock)
  • it doesn't protect C code from other C code (e.g. the FFI code may call both setenv and getenv itself, and that is unsafe even while Rust's lock is locked)

And it's not an easy or elegant solution:

  • It requires knowing about the problem and adding a workaround manually. It's hard to know which foreign code could potentially use env vars, so all FFI code would be suspect and it'd be an endless whack-a-mole.
  • It has a significant performance impact. It may end up serializing multi-threaded programs.
1 Like
23

I think an interesting solution would be to replace getenv/setenv with thread-safe implementations. The existing bad API could be made safe by leaking (interning) all the strings set in setenv, so that every pointer given out by getenv remained valid forever (fn getenv() -> &'static CStr).

Rust controls its own linking, so I guess that at least for statically-linked Rust programs it could replace its own calls to setenv/getenv with "leaky" wrappers. AFAIK it still can't do anything about calls in dynamically linked libraries, so mutable environment is going to remain a safety issue until it's fixed at the source — in libc.

1 Like
24

That thread also has this interesting quote (similar ideas came up here before):

Why don't you just bind the Lua setenv function to work with your own environment strings list that has nothing to do with the actual process's? This is the correct way to write a shell too -- it should never touch its own environment, but instead maintain shell variables itself and export them as needed to external programs it invokes.

In terms of backwards compatibility I tend to agree -- fixing all the existing getenv-using C code out there seems basically impossible, so probably making the "actual" environment immutable and only mutating a "shadow copy" is the only solution that has any chance of working.

Of course, it would be a breaking change for Rust to have env::set_var only mutate the "Rust shadow environment", not the real one...

I wonder what led to env::set_var even existing. These issues must have been already known 6 years ago, and in other places Rust was not afraid of diverging from "standard APIs" when it made sense (e.g. string indexing and the entire OsString situation).

2 Likes
25

I agree that there's not a good reason for env::set_var to exist in its current form. There are better ways to pass around mutable variables and even safe changes to the environment can be the cause of additional challenges (especially in multi-threaded programs). Seeing as set_var does exist, perhaps it should now be deprecated?

setenv can of course be necessary for certain FFI situations but then it may be better as an os specific function, given the differing guarantees for different platforms. Or perhaps just having it in the libc crate is enough.

1 Like
26

Wrapping?

  • write __wrap_getenv() to use an acquire read
  • write__wrap_setenv() to leak the old env and do a release write
  • stick -Wl,-wrap=getenv,-wrap=setenv somewhere

So long as GNU ld is in use it might work..

1 Like
27

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.