NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18.08k stars 14.06k forks source link

Purity problems with /bin/sh --- an example in NixPkgs #1424

Open 7c6f434c opened 10 years ago

7c6f434c commented 10 years ago

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?

vcunat commented 10 years ago

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

shlevy commented 10 years ago

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

domenkozar commented 10 years ago

Anyone up for the job for 14.11 release?

shlevy commented 10 years ago

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

lucabrunox commented 10 years ago

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

shlevy commented 10 years ago

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.

wmertens commented 10 years ago

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

lucabrunox commented 10 years ago

@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.

7c6f434c commented 10 years ago

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

What if SHELL is fish or even Python?

wmertens commented 10 years ago

@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...

7c6f434c commented 10 years ago

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

wmertens commented 10 years ago

@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" notifications@github.com 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.

lucabrunox commented 10 years ago

@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().

7c6f434c commented 10 years ago

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.

wmertens commented 10 years ago

@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" notifications@github.com 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.

lucabrunox commented 10 years ago

@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.

wmertens commented 10 years ago

@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" notifications@github.com 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.

7c6f434c commented 10 years ago

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

It will, via bash?

wmertens commented 10 years ago

More rewriting :grin:

Wout. (on phone, sorry for brevity) On Oct 22, 2014 11:08 PM, "Michael Raskin" notifications@github.com 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.

shlevy commented 10 years ago

@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).

shlevy commented 10 years ago

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

7c6f434c commented 10 years ago

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).

shlevy commented 10 years ago

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

7c6f434c commented 10 years ago

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

edolstra commented 10 years ago

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?

7c6f434c commented 10 years ago

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.

shlevy commented 10 years ago

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

shlevy commented 10 years ago

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

vcunat commented 10 years ago

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.

edolstra commented 10 years ago

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.

vcunat commented 10 years ago

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).

wmertens commented 10 years ago

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?

edolstra commented 10 years ago

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.)

bjornfor commented 10 years ago

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)?

vcunat commented 10 years ago

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.

edolstra commented 10 years ago

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

wmertens commented 10 years ago

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 notifications@github.com 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.

shlevy commented 10 years ago

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?

bjornfor commented 10 years ago

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).

shlevy commented 10 years ago

stdenv already depends on bash though...

bjornfor commented 10 years ago

@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.

shlevy commented 10 years ago

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

bjornfor commented 10 years ago

What's that?

$ man 3p system
No entry for system in section 3p of the manual
shlevy commented 10 years ago

Ah, part of pkgs.posix_man_pages

bjornfor commented 10 years ago

@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.)

shlevy commented 10 years ago

Sure, I'll do that.

7c6f434c commented 10 years ago

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…

vcunat commented 9 years ago

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

shlevy commented 9 years ago

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.

vcunat commented 9 years ago

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