Mitigate a segmentation fault when a derivation has no name attribute#5127
Closed
fogti wants to merge 1 commit intoNixOS:masterfrom
Closed
Mitigate a segmentation fault when a derivation has no name attribute#5127fogti wants to merge 1 commit intoNixOS:masterfrom
fogti wants to merge 1 commit intoNixOS:masterfrom
Conversation
Contributor
Author
|
@nixinator @happysalada could you try to test a nix with this patch applied on a derivation with a missing |
Contributor
|
Thank you for this, I'll add it to the list of things to do on monday. |
Contributor
|
I confirmed that it works. For anyone else, this can be tested in the following repo Thank you for the PR. |
Contributor
Author
|
@edolstra could you please review/merge this? |
Contributor
Author
|
hm, maybe I should adjust the commit message to be more accurate. original patch: commit b03b02775299fc29334cb0eba0e6c64e632f534c (HEAD -> iss5113, zseri/iss5113)
Author: zseri <zseri.devel@ytrizja.de>
Date: Thu Aug 12 16:25:22 2021 +0200
Try to partially mitigate a segmentation fault in libnixexpr.so when a derivation has no name attribute
diff --git a/src/libexpr/attr-set.hh b/src/libexpr/attr-set.hh
index 1da8d91df..2550ad032 100644
--- a/src/libexpr/attr-set.hh
+++ b/src/libexpr/attr-set.hh
@@ -41,7 +41,7 @@ private:
size_t size_, capacity_;
Attr attrs[0];
- Bindings(size_t capacity) : size_(0), capacity_(capacity) { }
+ Bindings(size_t capacity) : pos(&noPos), size_(0), capacity_(capacity) { }
Bindings(const Bindings & bindings) = delete;
public: |
…no name attribute This may not fix the underlying issue that the location data is not propagated early enough.
tomberek
approved these changes
Aug 21, 2021
Member
|
Seems related to #4895. |
Contributor
Author
|
yes. |
roberth
added a commit
to hercules-ci/nix
that referenced
this pull request
Aug 29, 2021
This fixes a class of crashes and introduces ptr<T> to make the code robust against this failure mode going forward. Thanks regnat for the idea of a ref<T> without overhead! Closes NixOS#4895 Closes NixOS#4893 Closes NixOS#5127 Closes NixOS#5113
Member
|
#5192 incorporates both and uses types to prevent it in the future. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5113, but does not fix the underlying issue that the location data is not propagated early enough (it might fix the segmentation fault, tho).