Fix segfault or stack overflow caused by large derivation fields #5875
Conversation
roberth
force-pushed
the
fix-large-drv-field-stack-overflow
branch
2 times, most recently
from
3f3c0a0 to
08ff118
Compare
|
Interestingly, the new test case triggers #3605 when run with a daemon, but not when the derivation field is modified to be "just" 1MiB. |
roberth
force-pushed
the
fix-large-drv-field-stack-overflow
branch
from
a57c1b0 to
7f7a2ab
Compare
|
I can confirm this fixes our Hydra Evaluator crashing. It would be nice to somehow fix that Darwin situation |
I think you were referring to a flaky test result in CI and I agree. |
|
@edolstra This is ready for review |
|
we've run the current head with decreased buffer size through our benchmarks and haven't seen a significant impact. 👍 |
|
@thufschmitt I believe this is ready to be merged. |
|
Could you add backport labels? |
roberth
force-pushed
the
fix-large-drv-field-stack-overflow
branch
from
961b680 to
2b3da07
Compare
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.
... to avoid non-standard, unidiomatic alloca.
Although this will leave gaps in the stack, the performance impact of those should be insignificant and we get a simpler solution this way.
roberth
force-pushed
the
fix-large-drv-field-stack-overflow
branch
from
2b3da07 to
dec7748
Compare
If we want to be careful about hitting the stack protector page, we should use `-fstack-check` instead. Co-authored-by: Eelco Dolstra <[email protected]>
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.
With this change, we avoid both types of failure.