Soundness issue with std::env::remove_var and other env manipulation in libsystemd

[mbuesch]

As described in the documentation, std::env::remove_var will become unsafe in a future version of Rust.

Even today the use of std::env::remove_var is unsound in multithreaded programs and can cause UB. In addition to that, env manipulation from non-Rust libs (libssytemd) can cause UB in Rust.

See the rust-lang/rust#27970 for more information.

I think this is a problem for rust-systemd, because threads can be accessing the environment, while rust-systemd calls remove_var or calls into libsystemd which changes the env.

What do you think?
Thanks for your opinions :)

[codyps]

I agree that is a problem that we'll want to take a look at in rust-systemd.

I suspect if there's a PR I'll work to merge it, but I don't spend much time on this crate at the moment.

[mbuesch]

Thanks for your reply.

Do you have a vague idea in what direction a fix could be?

I think this might be very hard to fix completely.
Even if all accesses are locked in Rust std (which is not the current plan, as far as I understand it), there can be unlocked C code that races with Rust code.

[codyps]

I suspect splitting the ListenFds::new(bool) into 2 functions where one is unsafe and the other is safe (and does not clear env vars). Also changing that fn to call the libsystemd function (probably) because the point of writing it ourselves was to avoid the env races.

I've taken a quick look at libsystemd itself, and it appears the only unsetting of env vars (unsetenv) is in this same sd-notify code, and there doesn't appear to be a place it's doing a setenv.