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

Add OpenOptions::exclusive #27217

Closed
wants to merge 1 commit into from

Conversation

sfackler
Copy link
Member

It maps to O_EXCL on *nix and CREATE_NEW on Windows.

r? @alexcrichton

@alexcrichton
Copy link
Member

What are the effects of having O_EXCL specified on unix without O_CREAT? It seems somewhat odd that .read(true).exclusive(true) can succeed on Windows.

Can you add a few more tests for the various flavors of flags as well? E.g. some cases like .read(true).exclusive(true) but also just throwing stuff in like .truncate(true) to make sure it doesn't mess with things.

@sfackler
Copy link
Member Author

The man page says that the behavior of O_EXCL is undefined if O_CREAT is not also passed, except for some corner case with block devices.

I can update the unix implementation to be a bit smarter and only tack on O_EXCL if create(true) and exclusive(true), or return an error in that case on both Windows and Unix, or we can just let the OS work it out. "Just let the OS work it out" seems fine to me personally - there's a similar situation of "silly" flag combinations with append(true).truncate(true) that I don't believe we handle specially.

@alexcrichton
Copy link
Member

We currently have another PR (#26772) to handle some corner cases like this (which is why I ask). We still have yet to make a decision on that PR, though, although I personally favor the "let's just call into the OS" route over "let's add some verification on our end" route.

@alexcrichton alexcrichton added I-needs-decision Issue: In need of a decision. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jul 22, 2015
It maps to O_EXCL on *nix and CREATE_NEW on Windows.
@sfackler sfackler force-pushed the open-options-exclusive branch from 443a489 to f1d6a50 Compare July 23, 2015 00:55
@pitdicker
Copy link
Contributor

As alexcrichton noted, how should differences between Windows and Unix be handled?
Just leave it to the os, always fail if exclusive(true) is used without create(true), or let exclusive(true) imply create(true)?
I prefer the last variant.

Can we have some bikeshedding about the name?
exclusive sounds to me like the locking Windows normally does, where one program can not write to a file while an other has it open.
Some alternatives:
create_new
must_not_exist

@pitdicker
Copy link
Contributor

Besides requiring that a file does not exist, O_EXCL has a special rule on Unix for dealing with symlinks.
This is to prevent a vulnerability where a privileged application would follow a symlink to for example /etc/passwd and overwrite it.
See https://lwn.net/Articles/250468/

This is not really a vulnerability on Windows, because it is somewhat difficult to create symlinks on it.
But if we want to guarantee the same behaviour on both platforms, it is easy to emulate on Windows:
Set flags_and_attributes to FILE_ATTRIBUTE_NORMAL | FILE_FLAG_OPEN_REPARSE_POINT

Just to make the behaviour of symlinks with O_EXCL clear, this is what happens when trying to open dir/file:

existing on system normal O_CREAT O_CREAT + O_EXCL
dir/ fails dir/file dir/file
dir/file dir/file dir/file fails (because dir/file exists)
dir/file->file2 (depends on file2) dir/file2 fails (because dir/file exists)
dir->dir2/ fails dir2/file dir2/file
dir->dir2/file dir2/file dir2/file fails (because dir2/file exists)
dir->dir2/file->file2 (depends on file2) dir2/file2 fails (because dir2/file exists)

I think this should be mentioned in the documentation, something like:
Nothing is allowed to exist on the target location, also no (dangling) symlink.

@alexcrichton
Copy link
Member

This will probably take a relatively similar outcome to #26772, and I wrote up some of the libs teams' thoughts here: #26772 (comment).

In general it may be the case that exclusive(true) implies create(true) instead.

@sfackler
Copy link
Member Author

I think I'll wait until #26772 resolves the behavior around implied flags and then update this.

@alexcrichton
Copy link
Member

I'm going to close this in favor of the associated RFC (rust-lang/rfcs#1252) for OpenOptions in general).

@sfackler sfackler deleted the open-options-exclusive branch November 26, 2016 05:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-needs-decision Issue: In need of a decision. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants