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

Rust 1.0.0 tarball does not have proper permissions on directories. #25479

Closed
hirigaray opened this issue May 16, 2015 · 11 comments
Closed

Rust 1.0.0 tarball does not have proper permissions on directories. #25479

hirigaray opened this issue May 16, 2015 · 11 comments
Labels
P-low Low priority T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.

Comments

@hirigaray
Copy link

The tarball in question: https://static.rust-lang.org/dist/rustc-1.0.0-src.tar.gz

I actually don't really understand what's going on, one +x is set, and I don't understand why +x is needed on directories, but it is.

I ran into a snag, even while operating as root. I got "permission denied" when installing:

cfg: version 1.0.0-dev (built 2015-05-15)
cfg: build triple x86_64-unknown-linux-gnu
cfg: host triples x86_64-unknown-linux-gnu
cfg: target triples x86_64-unknown-linux-gnu
cfg: host for x86_64-unknown-linux-gnu is x86_64
cfg: os for x86_64-unknown-linux-gnu is unknown-linux-gnu
cfg: good valgrind for x86_64-unknown-linux-gnu is 1
cfg: using CC=ccache gcc (CFG_CC)
cfg: enabling valgrind run-pass tests (CFG_ENABLE_VALGRIND_RPASS)
cfg: valgrind-rpass command set to "/usr/bin/valgrind" --error-exitcode=100 --soname-synonyms=somalloc=NONE --quiet --suppressions=/home/kori/local/build/rust/src/rustc-1.0.0/src/etc/x86.supp  --tool=memcheck --leak-check=full
cfg: no xelatex found, disabling LaTeX docs
cfg: no pandoc found, omitting PDF and EPUB docs
cfg: disabling doc build (CFG_DISABLE_DOCS)
cfg: including prepare rules
cfg: including dist rules
cfg: including install rules
make: stat: GNUmakefile: Permission denied
make: stat: makefile: Permission denied
make: stat: Makefile: Permission denied
make: stat: prepare_install: Permission denied
make: *** No rule to make target 'prepare_install'.  Stop.
/home/kori/local/build/rust/src/rustc-1.0.0/mk/install.mk:14: recipe for target 'install' failed
make: *** [install] Error 2

I did some digging, and I figured out the problem.

SOLUTION: Mode a+x needs to be set for all directories, so it works properly.

This is currently not the case.

EDIT: Apparently you need a+x to search for the files inside directories properly.

https://en.wikipedia.org/wiki/Chmod#Common_Errors

More detail: the dirs are currently drwx------, they should be at least drwx--x--x, but ideally drwxr-xr-x

@huonw
Copy link
Member

huonw commented May 16, 2015

cc @brson, @alexcrichton

@Stebalien
Copy link
Contributor

drwx------ (700) should work as long as you own the directories.

Unfortunately, when run under sudo (sudo make), the makefile detects that it is running under sudo and temporarily drops privileges to the original user (the one that called sudo) when building. This is fine in the sudo make case assuming the user that is calling sudo is the same user that extracted the archive (a reasonable assumption). However, if you extract the archive using sudo, the files will be owned by root and sudo make will fail because it tries to build as your user, not root.

IMO, the makefile should not try to be smart. If building as root is dangerous, the makefile should tell the user so and abort.

@hirigaray
Copy link
Author

While experimenting with this, I tried fixing the permissions, and I've run into yet another problem.

make prepare_install` works, but `make install` doesn't, it complains:
make: *** No rule to make target 'prepare_install'.  Stop.
/home/kori/local/build/rust/src/rustc-1.0.0/mk/install.mk:14: recipe for target 'install' failed
make: *** [install] Error 2

This doesn't make any sense.

@hirigaray
Copy link
Author

Yeah, I just took @Stebalien's tip and removed these lines: https://github.com/rust-lang/rust/blob/master/mk/install.mk#L12-15
EDIT: It works perfectly with this fix, and Stebalien's explanation is correct.

@brson
Copy link
Contributor

brson commented May 27, 2015

This may be a more general problem. AIUI permissions in tarballs are pretty broken because they encode local user id's that don't make any sense when distributed elsewhere. For this reason rust-installer explicitly sets the permissions of every single file during install (saw a similar bug there where files extracted from the rustc tarball and repackaged by rust-packaging did not maintain the right bits).

@brson
Copy link
Contributor

brson commented May 27, 2015

I'm curious though when this problems started happening. Our source tarball generation has barely changed ever. Has this always been a problem?

@brson
Copy link
Contributor

brson commented May 27, 2015

Nominating because one of our release artifacts seems to be completely broken.

@alexcrichton
Copy link
Member

triage: P-medium

Would love to fix, but doesn't seem super high priority right now.

@brson
Copy link
Contributor

brson commented Jan 27, 2016

FWIW, I hacked around this a long time ago in rust-installer by setting the permissions on everything at install time.

@alexcrichton alexcrichton added P-low Low priority and removed P-medium Medium priority labels Aug 22, 2016
@Mark-Simulacrum Mark-Simulacrum added T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. and removed T-tools labels May 24, 2017
@RalfJung
Copy link
Member

RalfJung commented May 31, 2017

Looking at https://static.rust-lang.org/dist/rustc-1.17.0-src.tar.gz and https://static.rust-lang.org/dist/rustc-1.16.0-src.tar.gz, this seems to be fixed, isn't it? I looked at a couple of directories, they all had rwxr-xr-x.

In fact, the code in rustup that works around this issue is nowadays actively harmful... see rust-lang/rustup#1140

@Mark-Simulacrum Mark-Simulacrum added T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue. and removed A-infrastructure labels Jun 25, 2017
@Mark-Simulacrum
Copy link
Member

Yes, I am fairly certain (checked a few days back) this was fixed in the move towards a Rusty rust-installer. Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-low Low priority T-dev-tools Relevant to the dev-tools subteam, which will review and decide on the PR/issue. T-infra Relevant to the infrastructure team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants