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

Fix segfault if a file from an invalid searchpath-entry is attempted to get built #3764

Closed
wants to merge 1 commit into from

Conversation

Ma27
Copy link
Member

@Ma27 Ma27 commented Jun 29, 2020

When I try to build an expression from a searchpath that happens to be
wrong, nix-build segfaults:

λ ma27 [~] → nix-build '<foo/bar>' -I foo=$(mktemp -d)
[1]    16252 segmentation fault (core dumped)  nix-build '<foo/bar>' -I foo=$(mktemp -d)

This happens if the file-Symbol in the Pos struct doesn't have a
value (due to the missing searchpath-entry). This causes a segfault
when the Error-implementation tries to cast Pos to an ErrPos.

This was most likely caused by 55eb717,
previously it was explicitly checked if pos has any value. This patch
basically switches back to this behavior by setting nixCode to
std::nullopt if pos is incomplete.

Also added a testcase to make sure that this doesn't appear again in the
future.

…to get built

When I try to build an expression from a searchpath that happens to be
wrong, `nix-build` segfaults:

```
λ ma27 [~] → nix-build '<foo/bar>' -I foo=$(mktemp -d)
[1]    16252 segmentation fault (core dumped)  nix-build '<foo/bar>' -I foo=$(mktemp -d)
```

This happens if the `file`-Symbol in the `Pos` struct doesn't have a
value (due to the missing searchpath-entry). This causes a segfault
when the `Error`-implementation tries to cast `Pos` to an `ErrPos`.

This was most likely caused by 55eb717,
previously it was explicitly checked if `pos` has any value. This patch
basically switches back to this behavior by setting `nixCode` to
`std::nullopt` if `pos` is incomplete.

Also added a testcase to make sure that this doesn't appear again in the
future.
@Ma27
Copy link
Member Author

Ma27 commented Jun 29, 2020

cc @edolstra @domenkozar @bburdette
We should also port this patch to flakes where I originally discovered this bug.

@bburdette
Copy link
Contributor

Good find. I came across something similar in my show-trace branch, in a different context. I've got a PR out for the general case of pos->errpos conversion, here: #3767

@edolstra
Copy link
Member

Is this PR still necessary after #3767?

@edolstra
Copy link
Member

edolstra commented Jul 1, 2020

Appears to work now.

@edolstra edolstra closed this Jul 1, 2020
@Ma27 Ma27 deleted the fix-invalid-search-path-files branch July 1, 2020 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants