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 broken pipe error on docker layer creation #101403
Conversation
Could you share a minimal example? Another way to solve it would be to figure out why we're getting Likely a command fails and the pipe is closed prematurely. |
I agree but SIGPIPE is rather a major security breech as is and should not be taken lightly : https://vigilance.fr/vulnerability/QEMU-denial-of-service-via-NBD-SIGPIPE-Signal-23103 |
SIGPIPE is merely a normal way for unix systems to communicate that the process behind a pipe is gone. It is not security problem in and of itself. The issue you've linked is a flaw in QEMU, which is a distinct program that is not involved in this PR. Other than that, the Nix sandbox should not be used for running untrustworthy code because it has not been audited (as far as I know). |
In fact both commands are working as expected because when run sequentially they finished properly. An interesting post which explains that well: https://stackoverflow.com/a/38549483/2165830 Maybe a better solution would be to ignore the broken pipe using the |
Here is the new patch which only add |
@utdemir looks like we need another approval before this can be merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please target release-20.09
, nixos-20.09
is a protected branch
To comply with CONTRIBUTING.md please have the commit message name be of the format
<pkg-name>: <subject-line>
for more examples, please look at https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md#submitting-changes
in your case, the commit message should be:
docker: Fix broken pipe error on layer creation
When backporting changes, please follow https://github.com/NixOS/nixpkgs/blob/master/.github/CONTRIBUTING.md#backporting-changes. Namely, you should be doing |
Sorry @jonringer I should have read the Contributing Guide before, thanks for the advice. I've change the commit to what you've suggested and rebase my contribution on master. I'll make another PR for the backport into 20.09. |
@mickours While figuring out what's going on with this PR, I noticed your commit does not have an author e-mail address that matches your GitHub account. You might want to fix that, either in https://github.com/settings/emails or in your git config and then Other than that, it looks good, but to the person who will merge this, let's alter the branch name in the merge commit message to something sensible. |
Add `-p` to the `tee` command to avoid exiting on breaking pipe due to tarsum finishing before tar which creating docker layers.
Done! |
Thank you! |
This reverts commit f12346d.
This commit was an optimization which was using pipes to read the layer tarball.
It was causing a 141 exit code (means SIGPIPE for
tar
) while running Layer packing in the VM lauched for the runAsRoot command.This error happen consistently on multiple servers running Ubuntu 18.04 servers with Nix 20.09.
I've tested the patch and the error disappeared on all servers.
Motivation for this change
Fix this error by removing the optimization, so it won't happen again.
Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)