Nixpkgs: Purity problems with /bin/sh --- an example in NixPkgs

Created on 29 Dec 2013  Β·  66Comments  Β·  Source: NixOS/nixpkgs

http://hydra.nixos.org/build/7380540/nixlog/1/raw

Symptom: SIGSEGV

Failed workarounds: build fails on Hydra, i.e. on NixOS with chroot builds.

Problem: build set LD_LIBRARY_PATH including glibc-2.18 /lib/. The make command used system() which called /bin/sh. /bin/sh is taken from something that existed at the beginning of the large build batch (either system or nix.conf). So it is linked against glibc-2.17. There seems to be some clash here...

This shows that system() impurity can bite us even under the ideal conditions: NixOS with chroot builds. Is it _now_ a good idea to patch glibc?

bug work-in-progress nixos reproducible builds

All 66 comments

Yes, I agree that system() should not use /bin/sh.

Won't be able to fix this in time for 14.04, but hopefully someone can implement looking up sh in PATH by 14.10

Anyone up for the job for 14.11 release?

@edolstra Strong objection to making system and popen etc. look up sh in PATH before falling back to /bin/sh?

Will bash run in sh compatibility mode if not run as /bin/sh? I fear also bash has to be patched.

We can set argv[0] to /bin/sh I suppose, if that turns out to be the problem. /proc/self/exe points to the fully resolved path, so there's no other way bash could know.

How about using $SHELL and if that's not there using /bin/sh?

@wmertens if that's only to avoid a glibc rebuild, well a bash rebuild still triggers a glibc rebuild. At this point, better make it pure once for all.

How about using $SHELL and if that's not there using /bin/sh?

What if SHELL is fish or even Python?

@lethalman I would be happy with a pure path, I just wanted to avoid walking $PATH on system() calls. Other than that, doesn't that give us a circular dependency?

@7c6f434c the buildenv should set $SHELL...

@wmertens But system() is also used in user applications with expectation that SHELL=python doesn't really truly break it

@7c6f434c Oops you're quite correct...

Likewise an sh in the path could cause unexpected side effects...

Wout.
(on phone, sorry for brevity)
On Oct 22, 2014 10:35 PM, "Michael Raskin" [email protected] wrote:

@wmertens https://github.com/wmertens But system() is also used in user
applications with expectation that SHELL=python doesn't really truly break
it

β€”
Reply to this email directly or view it on GitHub
https://github.com/NixOS/nixpkgs/issues/1424#issuecomment-60150853.

@wmertens glibc already builds with stdenv and bash... it's a bootstrap non-interactive shell of course (AFAIK), but it should serve well to be used for system().

Completely-non-sh sh in PATH is less likely than incompatible $SHELL, I hope.

@lethalman I guess some people argue for being able to GC the input bash after glibc build.

@7c6f434c an option would be to rewrite the bash references in glibc to the
interactive bash... Closing the loop.

Wout.
(on phone, sorry for brevity)
On Oct 22, 2014 10:48 PM, "Michael Raskin" [email protected] wrote:

Completely-non-sh sh in PATH is less likely than incompatible $SHELL, I
hope.

@lethalman https://github.com/lethalman I guess some people argue for
being able to GC the input bash after glibc build.

β€”
Reply to this email directly or view it on GitHub
https://github.com/NixOS/nixpkgs/issues/1424#issuecomment-60152741.

@wmertens now you have two glibc instead of two bash :) I think bash should do no wild guess about what's /bin/sh. If it's not sh-compatible, runtime applications break anyway. So just force it to run bash in sh-compatible mode.
Alternatively, dash.

@lethalman but the second glibc wouldn't refer to the first one so the
first one could be garbage collected?

Wout.
(on phone, sorry for brevity)
On Oct 22, 2014 11:01 PM, "lethalman" [email protected] wrote:

@wmertens https://github.com/wmertens now you have two glibc instead of
two bash :)

β€”
Reply to this email directly or view it on GitHub
https://github.com/NixOS/nixpkgs/issues/1424#issuecomment-60154664.

@lethalman but the second glibc wouldn't refer to the first one so the
first one could be garbage collected?

It will, via bash?

More rewriting :grin:

Wout.
(on phone, sorry for brevity)
On Oct 22, 2014 11:08 PM, "Michael Raskin" [email protected] wrote:

@lethalman but the second glibc wouldn't refer to the first one so the
first one could be garbage collected?

It will, via bash?

β€”
Reply to this email directly or view it on GitHub
https://github.com/NixOS/nixpkgs/issues/1424#issuecomment-60155729.

@wmertens You can't break the loop unless you make a derivation that contains both bash and glibc. At one time I proposed just that, using multiple outputs so that each could be in its own path, but it would require allowing cycles in the reference graph (technically possible, just not currently allowed in nix).

IMO anyone setting sh to a non-POSIX shell deserves what they get.

IMO anyone setting sh to a non-POSIX shell deserves what they get.

Please read my original report; there is a problem on NixOS with bash
/bin/sh (under specific conditions).

@7c6f434c I meant in PATH, for nix builds sh in PATH will always point to the current bash.

Ah. PATH seems to be least bad solution, I agree

I don't see what @7c6f434c's example has to do with the use of /bin/sh. If system() called sh in $PATH while there is a conflicting $LD_LIBRARY_PATH, couldn't you get exactly the same problem?

I don't see what @7c6f434c's example has to do with the use of /bin/sh. If system() called sh in $PATH while there is a conflicting $LD_LIBRARY_PATH, couldn't you get exactly the same problem?

With PATH, you'd get _new_ sh linked against new glibc.

Hmm, I wonder if we can't separate system and popen into a separate library that is built _after_ bash...

(unless bash itself uses those, which would surprise me...)

I guess that this idea is out of question: bundling the basic bash and glibc into one output path?

The thing is that glibc and bash use each other. (Similar was discussed for libgcc_s as well.) Implementation could e.g. build both separately, perhaps with adjusted package names, and then combine them by hash rewriting into one output.

Or copy the bash from bootstrap-tools and use patchelf to adjust it to the new Glibc. Alternatively, build a new bash using the bootstrap-tools before Glibc, then in Glibc's derivation, copy it to $out/bin and use patchelf.

Of course, it would include hardcoding the bash path in glibc to that point. As you say, copying bash and re-patchelfing it should work, and it's a nicer solution (assuming bash uses glibc only in RPATH).

Yeah so a glibc that includes bash seems to be the cleanest.

Then to optimize disk space and memory page reuse, glibc should include
the current bashInteractive and bashInteractive should just be glibc.

So on Darwin, we're not using glibc and therefore they would stay separate,
right?

No, it should not be bashInteractive, because then we'll need readline and ncurses. (Also, not depending on those may make sh start slightly faster.)

IIRC, Ubuntu switched /bin/sh from bash to dash because of speed reasons. Any reason not to have glibc system() etc. use dash instead of bash (assuming there is some speed to gain)?

Yes, Debian did that, but IIRC it was mainly to speed startup which was handled by shell scripts. AFAIK we use systemd for most of that work.

The main reason not to do that is that it will undoubtedly break lots of shell scripts.

I'm not so sure... if you call system() you know you're getting sh
semantics. Expecting anything else is a bug. If you run a script, it will
be called with its script interpreter, which might be bash. No?

On Thu, Oct 23, 2014 at 2:09 PM, Eelco Dolstra [email protected]
wrote:

The main reason not to do that is that it will undoubtedly break lots of
shell scripts.

β€”
Reply to this email directly or view it on GitHub
https://github.com/NixOS/nixpkgs/issues/1424#issuecomment-60229282.

OK, I have in progress work here. Basically, during the bootstrap, we'll:

  1. build final glibc, named glibc-and-bash-${some-version-string} with system and friends pointing to $out/bin/sh
  2. build final bash, named the same as final glibc
  3. have a knot-tying derivation that:

    1. copies the final glibc to $out, rewriting references to glibc to $out

    2. removes $out/bin/sh

    3. copies the final bash to $out, rewriting references to glibc and bash to $out

The result of the knot-tying derivation will then be used as _both_ bash and glibc for every other package in the final stdenv/pkgs set.

Objections?

I think @wmertens is right. I'd change "glibc-and-bash" to "glibc-and-shell" (or "sh") and then try the "dash" shell. Expecting system() to use bash is a bug, even more so than expecting /bin/sh to be bash (IMHO).

stdenv already depends on bash though...

@shlevy: Ok, not a big deal.

Also, I just read "man 3 system":

 The  system()  library  function uses fork(2) to create a child process
       that executes the shell command specified in command using execl(3)  as
       follows:

           execl("/bin/sh", "sh", "-c", command, (char *) 0);

So I was wrong by thinking there was a difference between system() and /bin/sh.

IMO man 3p system is the one you want :wink:

What's that?

$ man 3p system
No entry for system in section 3p of the manual

Ah, part of pkgs.posix_man_pages

@shlevy: Thanks. +1 for the approach you describe above. (Although I'd prefer if it was named "glibc-and-shell", even though shell will be bash for the foreseeable future.)

Sure, I'll do that.

Given that we have some support for creating quite strange environments in NixPkgs, even if shell choice is unusable for general purpose installations someone will use it…

@shlevy: any progress? Or should I look into it?

Ongoing work at https://github.com/shlevy/nixpkgs/tree/glibc-with-shell , testing the latest now and will open a PR if all is well.

Agreement on a solution isn't so easy, which seems to make this unrealistic for 14.11.

Having thought about it a bit, I think the least intrusive/ugly solution is to have system() and popen() look for sh in $PATH, but only if we're in a Nix build. So basically to apply patch [1], but conditional at runtime on some environment variable like $NIX_STORE. This variable would have to be added to sysdeps/generic/unsecvars.h to prevent problems with setuid binaries. This approach would get rid of the dependency on /bin/sh in Nix builds, while circumventing the cyclic dependency between glibc and bash.

We would have to do something similar for gawk [2].

[1] https://gitorious.org/guix/guix/source/5d4fd2671aaadc5b0b7e0c331fa5e2a1d7e5c4d8:distro/packages/patches/glibc-bootstrap-system.patch
[2] https://gitorious.org/guix/guix/source/5d4fd2671aaadc5b0b7e0c331fa5e2a1d7e5c4d8:distro/packages/patches/gawk-shell.patch

@edolstra so during a build it calls sh and otherwise /bin/sh? Won't
that give purity problems outside builds (as well as hard-to-debug issues
when a different sh ends up in the path)?

If not /bin/sh, how do we get the full path to sh in glibc?
On Tue Jan 27 2015 at 12:17:06 PM Eelco Dolstra [email protected]
wrote:

Having thought about it a bit, I think the least intrusive/ugly solution
is to have system() and popen() look for sh in $PATH, but only if we're
in a Nix build. So basically to apply patch [1], but conditional at runtime
on some environment variable like $NIX_STORE. This variable would have to
be added to sysdeps/generic/unsecvars.h to prevent problems with setuid
binaries. This approach would get rid of the dependency on /bin/sh in Nix
builds, while circumventing the cyclic dependency between glibc and bash.

We would have to do something similar for gawk [2].

[1]
https://gitorious.org/guix/guix/source/5d4fd2671aaadc5b0b7e0c331fa5e2a1d7e5c4d8:distro/packages/patches/glibc-bootstrap-system.patch
[2]
https://gitorious.org/guix/guix/source/5d4fd2671aaadc5b0b7e0c331fa5e2a1d7e5c4d8:distro/packages/patches/gawk-shell.patch

β€”
Reply to this email directly or view it on GitHub
https://github.com/NixOS/nixpkgs/issues/1424#issuecomment-71630803.

Yes, so this would only change behaviour during builds.

I don't know, somehow using sh from PATH seems more troublesome than
creating a glibc+bash package (an approach that might also work on Darwin's
libSystem).

I was also thinking that it wouldn't solve
https://github.com/NixOS/nixpkgs/issues/2835 but that's just a matter of
running patchShebangs.

So basically I can't come up with any real objections :)

On Tue Jan 27 2015 at 1:14:00 PM Eelco Dolstra [email protected]
wrote:

Yes, so this would only change behaviour during builds.

β€”
Reply to this email directly or view it on GitHub
https://github.com/NixOS/nixpkgs/issues/1424#issuecomment-71638671.

Please read https://github.com/NixOS/nixpkgs/pull/4998#issuecomment-71816223 and comment here or there.

I have another approach to the glibc problem (that I was considering implementing for libSystem on darwin):

  1. Patch your libc to not mention /bin/sh ,but instead to call a function (or just look at an external symbol) to find out what shell to use, and allow undefined symbols during linkage. Call this package rawLibc or something like it.
  2. Have bash depend on rawLibc, but patch it to define the missing symbol for the shell, pointing to itself.
  3. Make a wrapper package around rawLibc that depends on bash as well, and also defines the missing symbol to point at the bash in question. It then re-exports libc + the extra symbol and pretends to be a complete libc. On darwin, we have explicit re-exports of symbols, and on linux the glibc so can just be a linker script that points to rawLibc and the little shim that defines the symbol.

(triage) this looks like a bigger design problem, has a solution been found in the meantime?

I might have a general solution with https://github.com/NixOS/nix/issues/1080

@Ericson2314 https://github.com/NixOS/nixpkgs/pull/4998 already did basically your suggestion there, except all in one output instead of multiple outputs.

@shlevy yes in this case they are equivalent and I wish that had happened.

What is the status with this? Is this still unsolved?

Well, our glibc still includes a reference to /bin/sh

I dunno, maybe builds should be allowed to override parts of the chroot environment with store paths?

Thank you for your contributions.

This has been automatically marked as stale because it has had no activity for 180 days.

If this is still important to you, we ask that you leave a comment below. Your comment can be as simple as "still important to me". This lets people see that at least one person still cares about this. Someone will have to do this at most twice a year if there is no other activity.

Here are suggestions that might help resolve this more quickly:

  1. Search for maintainers and people that previously touched the related code and @ mention them in a comment.
  2. Ask on the NixOS Discourse.
  3. Ask on the #nixos channel on irc.freenode.net.

Well, this issue will probably still bite us but rarely.

One more option, I guess: sandbox could use a /bin/sh being a statically linked binary delegating to the sh mentioned in $NIX_BIN_SH, with build-time stdenv's sh as the default. As it would be used only in the build sandbox, horrible setuid secuity issues with this approach are moot. And then stdenv.mkDerivation could make sure that stdenv.shell is used just by setting a variable.

Well, this issue will probably still bite us but rarely.

For non-NixOS users, I think more than rarely.

At least it was annoying for me a year ago when I tried to run Nix-built programs with LD_LIBRARY_PATH wrappers and saw system() calls to Ubuntu's /bin/sh. The program crashed. (I don't remember if or how I fixed it.)

Well, this issue will probably still bite us but rarely.
For non-NixOS users, I think more than rarely.

Well, this issue is only distinct from all the other /bin/sh purity discussions by being a problem even during a sandboxed build. Runtime problems often (but not always) are fixable with some namespace-based tricks like FHS envs.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

worldofpeace picture worldofpeace  Β·  103Comments

nh2 picture nh2  Β·  76Comments

globin picture globin  Β·  65Comments

grahamc picture grahamc  Β·  77Comments

ttuegel picture ttuegel  Β·  98Comments