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

Standard library masks away bits in permissions struct #44147

Closed
tmerr opened this issue Aug 29, 2017 · 3 comments
Closed

Standard library masks away bits in permissions struct #44147

tmerr opened this issue Aug 29, 2017 · 3 comments
Labels
C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@tmerr
Copy link
Contributor

tmerr commented Aug 29, 2017

Behavior

On linux, mkdir mydir, chmod 2777 mydir then run:

fn main() {
  let path = "mydir";
  let perms = std::fs::metadata(path).unwrap().permissions();
  std::fs::set_permissions(path, perms).unwrap();
}

This changes the permissions of mydir to 777. Is this expected?

The cause

Metadata.permissions() masks away everything other than the lower 3 triads when it calls out to this code:

pub fn perm(&self) -> FilePermissions {
FilePermissions { mode: (self.stat.st_mode as mode_t) & 0o777 }
}

Then std::fs::set_permissions sends these mode bits into into chmod which considers four triads, not three. Possibly useful references: chmod and mode_t

Should we change Metadata.permissions to preserve that triad, maybe by masking with 0o7777 instead of 0o777?

@shepmaster shepmaster added C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Sep 1, 2017
@tmerr
Copy link
Contributor Author

tmerr commented Sep 10, 2017

I guess the solution depends on whether Unix's std::fs::Permissions is supposed to care about 9 bits or 12 (this is not an implementation detail since it's exposed to the programmer via Permissions.mode() of PermissionsExt). @alexcrichton any thoughts on this?

@sfackler
Copy link
Member

It seems to me like we should extend the mask to the full 12 bits.

@alexcrichton
Copy link
Member

Seems reasonable to me to avoid masking where it makes sense!

tmerr added a commit to tmerr/rust that referenced this issue Sep 16, 2017
Most users would expect set_permissions(Metadata.permissions()) to be
non-destructive. While we can't guarantee this, we can at least pass
the needed info to chmod.

Also update the PermissionsExt documentation to disambiguate what it
contains, and to refer to the underlying value as `st_mode` rather than
its type `mode_t`.

Closes rust-lang#44147
bors added a commit that referenced this issue Sep 22, 2017
Retain suid/sgid/sticky bits in Metadata.permissions

Most users would expect set_permissions(Metadata.permissions()) to be
non-destructive. While we can't guarantee this, we can at least pass
the needed info to chmod.

Also update the PermissionsExt documentation to disambiguate what it
contains, and to refer to the underlying value as `st_mode` rather than
its type `mode_t`.

Closes #44147
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

4 participants