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 assumes PATH_MAX is not a lie in a few places #27454

Closed
sstewartgallus opened this Issue Aug 1, 2015 · 0 comments

Comments

Projects
None yet
2 participants
@sstewartgallus
Copy link
Contributor

sstewartgallus commented Aug 1, 2015

See http://insanecoding.blogspot.ca/2007/11/pathmax-simply-isnt.html for some background. Basically, PATH_MAX just doesn't work.

Currently. Rust uses PATH_MAX in a few important places.

./src/libstd/sys/unix/fs.rs:377:let mut buf = vec![0;libc::PATH_MAX as usize];
./src/libstd/sys/unix/fs.rs:469:len = 1024; // FIXME: read PATH_MAX from C ffi?
./src/rt/rust_builtin.c:337:char buf[2*PATH_MAX], exe[2*PATH_MAX];
./src/rt/rust_builtin.c:380:snprintf(buf, 2*PATH_MAX, "%s/%s", paths[i], argv0);

Generally, the best approach to work around this is to loop and double a buffer in size until it is big enough in size to fill with the path or to query the size of the path of the file using fpathconf. I'm not sure if any filesystems on any modern OSes return -1 for the result of fpathconf. Letting things get too big opens up a DOS attack though so be careful I guess?

The fpathconf strategy or the doubling buffer strategy can be done with ./src/libstd/sys/unix/fs.rs:469 but I'm not sure how to handle the Rust cases that use realpath (unless realpath is reimplemented by hand). The C code can just use the NULL argument but I forget if there's a way to martial allocated by C data to Rust.

barosl added a commit to barosl/rust that referenced this issue Aug 21, 2015

Rewrite `std::sys::fs::readlink` not to rely on `PATH_MAX`
It currently has the following problems:

1. It uses `_PC_NAME_MAX` to query the maximum length of a file path in
the underlying system. However, the meaning of the constant is the
maximum length of *a path component*, not a full path. The correct
constant should be `_PC_PATH_MAX`.

2. `pathconf` *may* fail if the referred file does not exist. This can
be problematic if the file which the symbolic link points to does not
exist, but the link itself does exist. In this case, the current
implementation resorts to the hard-coded value of `1024`, which is not
ideal.

3. There may exist a platform where there is no limit on file path
lengths in general. That's the reaon why GNU Hurd doesn't define
`PATH_MAX` at all, in addition to having `pathconf` always returning
`-1`. In these platforms, the content of the symbolic link can be
silently truncated if the length exceeds the hard-coded limit mentioned
above.

4. The value obtained by `pathconf` may be outdated at the point of
actually calling `readlink`. This is inherently racy.

This commit introduces a loop that gradually increases the length of the
buffer passed to `readlink`, eliminating the need of `pathconf`.

Fixes rust-lang#27454.

barosl added a commit to barosl/rust that referenced this issue Aug 21, 2015

Remove the arbitrary memory limit of `std::sys::fs::realpath`
As per POSIX 2013, `realpath` will return a malloc'ed buffer if the
second argument is a null pointer.[1]

Fixes rust-lang#27454.

[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/realpath.html

barosl added a commit to barosl/rust that referenced this issue Aug 21, 2015

Comment on functions that are still using `PATH_MAX`
There are some functions that only work in terms of `PATH_MAX`, such as
`F_GETPATH` in OS X. Comments on them for posterity.

Fixes rust-lang#27454.

bors added a commit that referenced this issue Aug 27, 2015

Auto merge of #27930 - barosl:path_max, r=alexcrichton
This PR rewrites the code that previously relied on `PATH_MAX`.

On my tests, even though the user gives the buffer length explicitly, both Linux's glibc and OS X's libc seems to obey the hard limit of `PATH_MAX` internally. So, to truly remove the limitation of `PATH_MAX`, the related system calls should be rewritten from scratch in Rust, which this PR does not try to do.

However, eliminating the need of `PATH_MAX` is still a good idea for various reasons, such as: (1) they might change the implementation in the future, and (2) some platforms don't have a hard-coded `PATH_MAX`, such as GNU Hurd.

More details are in the commit messages.

Fixes #27454.

r? @alexcrichton

@bors bors closed this in #27930 Aug 27, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.