Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

musl: 1.1.11 -> 1.1.15, add security patch. #21023

Merged
merged 1 commit into from
Dec 16, 2016
Merged

Conversation

dtzWill
Copy link
Member

@dtzWill dtzWill commented Dec 9, 2016

Motivation for this change

Update to latest (Aug 2015 -> July 2016), and apply recommended upstream patch for security issue present in all versions.

(see http://www.openwall.com/lists/oss-security/2016/10/19/1 for more information)

Things done
  • Tested using sandboxing
    (nix.useSandbox on NixOS,
    or option build-use-sandbox in nix.conf
    on non-NixOS)
  • Built on platform(s)
    • NixOS
    • macOS
    • Linux
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

I did test using nox-review but it only built the musl package. I'm not sure if there are more useful things to check that use musl.

@mention-bot
Copy link

@dtzWill, thanks for your PR! By analyzing the history of the files in this pull request, we identified @thoughtpolice, @wkennington and @obadz to be potential reviewers.

@dtzWill
Copy link
Member Author

dtzWill commented Dec 9, 2016

Well I tried checking the bootstrap tools on Linux, but with or without this change I'm unable to evaluate 'test' attribute:

$ nix-build pkgs/top-level/release.nix -A stdenvBootstrapTools.x86_64-linux.test -Q --show-trace
error: while evaluating the attribute ‘buildCommand’ of the derivation ‘test-bootstrap-tools’ at /home/will/misc/2016/dec/nixpkgs-musl/pkgs/stdenv/linux/make-bootstrap-tools.nix:188:5:
while evaluating anonymous function at /home/will/misc/2016/dec/nixpkgs-musl/pkgs/stdenv/linux/default.nix:6:1, called from /home/will/misc/2016/dec/nixpkgs-musl/pkgs/stdenv/linux/make-bootstrap-tools.nix:176:21:
assertion failed at /home/will/misc/2016/dec/nixpkgs-musl/pkgs/stdenv/linux/make-bootstrap-tools.nix:183:19

Quick investigation suggests this is due to 5c6234a, although maybe I'm invoking things wrong under the new system.

Anyway, looks like 'dist' seems to work for both x86_64-linux and i686-linux, with the contained 'busybox' binary seemingly working fine.

@dtzWill
Copy link
Member Author

dtzWill commented Dec 15, 2016

The evaluation errors above are resolved in #21189 .

Rebasing onto that gets test working for x86_64-linux and i686-linux! 🎉

@fpletz fpletz merged commit be24f1d into NixOS:master Dec 16, 2016
@dtzWill dtzWill deleted the update/musl branch December 17, 2016 21:42
@apatil
Copy link

apatil commented Jan 25, 2017

@dtzWill, sorry for commenting on an old thread but what was the reason for adding --disable-gcc-wrapper in https://github.com/NixOS/nixpkgs/pull/21023/files#diff-6419c747d225d6615bb68bce4fcf42ecR25 ? I didn't see it mentioned in the CVE.

@dtzWill
Copy link
Member Author

dtzWill commented Jan 26, 2017

It's not related to the CVE, but was added as part of upgrading to 1.1.14 as originally done in another PR: https://github.com/NixOS/nixpkgs/pull/16217/files#diff-6419c747d225d6615bb68bce4fcf42ecR23 .

Removing that flag doesn't seem to change behavior of normal build (it decides to not build any wrappers) I'll see what happens when cross-compiling.

Are you looking to enable and install the wrapper?

@dtzWill
Copy link
Member Author

dtzWill commented Jan 26, 2017

Well I was mistaken my current big evaluation rebuilds compilers and binutils and all sorts of things... but only musl the once!

Was hoping I could tease out a reason for the change in terms of breakage but seems to be fine for me :).

I think the idea is that we'll provide our own cc-wrapper (or gcc-cross-wrapper) anyway and so musl shouldn't bother.

Hope that helps at least explain the history and reasoning of the change.

@apatil
Copy link

apatil commented Jan 26, 2017

Thanks for looking into this @dtzWill. It's no huge deal, but I would vote for leaving off --disable-gcc-wrapper and letting musl build musl-gcc. It's handy for building static binaries. When I commented that line out locally and nix-env installed musl, I did get musl-gcc.

@dtzWill
Copy link
Member Author

dtzWill commented Jan 26, 2017

Hmm, my tree (including some of the expression for musl) has some differences that would explain the difference no worries. I mean sounds good to me either way, I don't have a particular preference so I'd err on going with the answer that works best for everyone-- which sounds like removing it!

Would you mind submitting a PR to do so? :)

rasendubi added a commit to rasendubi/nixpkgs that referenced this pull request Jan 27, 2017
The build of the wrapper was disabled in 93e44be (NixOS#21023) and is not
related to the CVE itself. (See comments in the mentioned PR.)
@rasendubi rasendubi mentioned this pull request Jan 27, 2017
7 tasks
@rasendubi
Copy link
Member

Submitted a PR: #22212.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants