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

Previously-present output not removed before rebuilding multiple-output derivation #122

Closed
civodul opened this issue May 23, 2013 · 9 comments

Comments

@civodul
Copy link
Contributor

civodul commented May 23, 2013

Suppose you have a multiple-output derivation, and one of its output is already present in the store.

When re-building the derivation, the already-present output is not present.

This leads to an assertion failure at "assert(!S_ISDIR(st.st_mode));" in canonicalisePathMetaData_ because the owner of the already-present output is `root', not the build user.

A possible fix would be to remove any already-present outputs before rebuilding the derivation.

@edolstra
Copy link
Member

Already-present outputs cannot be removed in general because they might be live.

We should simply not canonicalize outputs that were already valid.

@civodul
Copy link
Contributor Author

civodul commented Jun 8, 2013

@vcunat
Copy link
Member

vcunat commented Jun 13, 2013

IMO this fails before computeClosure is even called. The derivation would try to install into a path to which it has no permissions. Either you are able to garbage-collect the obstructing path, or I don't see there could be a good way to fix this problem (except for switching to intensional store, but that would be a really big change).

@civodul
Copy link
Contributor Author

civodul commented Jun 13, 2013

Vladimír Čunát notifications@github.com skribis:

IMO this fails before computeClosure is even called. The derivation would try to install into a path to which it has no permissions. Either you are able to garbage-collect the obstructing path, or I don't see there could be a good way to fix this problem (except for switching to intensional store, but that would be a really big change).

The patch I posted has been tested and fixes the problem AFAICS.

Ludo’.

@edolstra
Copy link
Member

@vcunat That's not a problem because Nix either performs the build in a chroot (where the obstructing paths are not present) or performs magic hash rewriting.

edolstra added a commit that referenced this issue Jun 13, 2013
The assertion in canonicalisePathMetaData() failed because the
ownership of the path already changed due to the hash rewriting.  The
solution is not to check the ownership of rewritten paths.

Issue #122.
@edolstra
Copy link
Member

@civodul I've committed a different fix, namely to let computeClosure() skip paths that were already valid. If I'm not mistaken, your fix has a security hole in that it would allow a build that does "ln /etc/shadow $out" (on systems without hard link restrictions). (There actually is a similar security bug in hash rewriting, see cd49ee0.)

@vcunat
Copy link
Member

vcunat commented Jun 13, 2013

@edolstra: Ah, I forgot chroots. Is hash rewriting used somewhere (or some analogue of intensional store from your thesis)?

@edolstra
Copy link
Member

Yes, it's used when chroots are disabled/unsupported. You can see it in action if you build a multiple-output derivation, garbage-collect one of the outputs, and then build the derivation again.

@civodul
Copy link
Contributor Author

civodul commented Jun 13, 2013

Wonderful, thanks!

zolodev pushed a commit to zolodev/nix that referenced this issue Jan 1, 2024
….5.1

chore(deps): bump sphinx from 3.4.3 to 3.5.1
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

No branches or pull requests

3 participants