From d038a67bd3c6ed0d6452d595cf0af3115e14c48f Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 6 Jan 2022 01:20:12 +0100 Subject: [PATCH] Fix segfault or stack overflow caused by large derivation fields This removes a dynamic stack allocation, making the derivation unparsing logic robust against overflows when large strings are added to a derivation. Overflow behavior depends on the platform and stack configuration. For instance, x86_64-linux/glibc behaves as (somewhat) expected: $ (ulimit -s 20000; nix-instantiate tests/lang/eval-okay-big-derivation-attr.nix) error: stack overflow (possible infinite recursion) $ (ulimit -s 40000; nix-instantiate tests/lang/eval-okay-big-derivation-attr.nix) error: expression does not evaluate to a derivation (or a set or list of those) However, on aarch64-darwin: $ nix-instantiate big-attr.nix ~ zsh: segmentation fault nix-instantiate big-attr.nix This indicates a slight flaw in the single stack protection page approach that is not encountered with normal stack frames. --- src/libstore/derivations.cc | 10 +++++++++- tests/big-derivation-attr.nix | 13 +++++++++++++ tests/simple.sh | 6 ++++++ 3 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 tests/big-derivation-attr.nix diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index b926bb711..616e78076 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -272,7 +272,15 @@ Derivation parseDerivation(const Store & store, std::string && s, std::string_vi static void printString(string & res, std::string_view s) { - char buf[s.size() * 2 + 2]; + char * buf; + size_t bufSize = s.size() * 2 + 2; + std::unique_ptr dynBuf; + if (bufSize < 0x10000) { + buf = (char *)alloca(bufSize); + } else { + dynBuf = decltype(dynBuf)(new char[bufSize]); + buf = dynBuf.get(); + } char * p = buf; *p++ = '"'; for (auto c : s) diff --git a/tests/big-derivation-attr.nix b/tests/big-derivation-attr.nix new file mode 100644 index 000000000..35c1187f6 --- /dev/null +++ b/tests/big-derivation-attr.nix @@ -0,0 +1,13 @@ +let + sixteenBytes = "0123456789abcdef"; + times16 = s: builtins.concatStringsSep "" [s s s s s s s s s s s s s s s s]; + exp = n: x: if n == 1 then x else times16 (exp (n - 1) x); + sixteenMegabyte = exp 6 sixteenBytes; +in +assert builtins.stringLength sixteenMegabyte == 16777216; +derivation { + name = "big-derivation-attr"; + builder = "/x"; + system = "y"; + bigAttr = sixteenMegabyte; +} diff --git a/tests/simple.sh b/tests/simple.sh index 15bd2bd16..50d44f93f 100644 --- a/tests/simple.sh +++ b/tests/simple.sh @@ -25,3 +25,9 @@ if test "$outPath" != "/foo/lfy1s6ca46rm5r6w4gg9hc0axiakjcnm-dependencies.drv"; echo "hashDerivationModulo appears broken, got $outPath" exit 1 fi + +outPath="$(NIX_REMOTE=local?store=/foo\&real=$TEST_ROOT/real-store nix-instantiate --readonly-mode big-derivation-attr.nix)" +if test "$outPath" != "/foo/xxiwa5zlaajv6xdjynf9yym9g319d6mn-big-derivation-attr.drv"; then + echo "big-derivation-attr.nix hash appears broken, got $outPath. Memory corruption in large drv attr?" + exit 1 +fi