Skip to content

mount/linux: add a safe wrapper for open_tree(2) #1958

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,9 @@ This project adheres to [Semantic Versioning](https://semver.org/).
- Added `F_KINFO` FcntlFlags entry on FreeBSD for `::nix::fcntl`.
([#2152](https://github.com/nix-rust/nix/pull/2152))

- Added `mount::open_tree()` helper on Linux.
([#1958](https://github.com/nix-rust/nix/pull/1958))

Comment on lines +45 to +47
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have changed our changelog mode in #2149 (the old one is way too conflict-prone), please see CONTRIBUTING.md on how to add a changelog

### Changed

- The MSRV is now 1.69
Expand Down
44 changes: 42 additions & 2 deletions src/mount/linux.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use crate::errno::Errno;
use crate::fcntl::at_rawfd;
use crate::{NixPath, Result};
use libc::{self, c_int, c_ulong};
use libc::{self, c_int, c_uint, c_ulong};
use std::os::unix::io::{AsFd, AsRawFd, FromRawFd, OwnedFd, RawFd};

libc_bitflags!(
/// Used with [`mount`].
Expand Down Expand Up @@ -85,7 +87,7 @@ libc_bitflags!(
MNT_DETACH;
/// Mark the mount point as expired.
MNT_EXPIRE;
/// Don't dereference `target` if it is a symlink.
/// Don't dereference `target` if it is a symlink.
UMOUNT_NOFOLLOW;
}
);
Expand Down Expand Up @@ -161,3 +163,41 @@ pub fn umount2<P: ?Sized + NixPath>(target: &P, flags: MntFlags) -> Result<()> {

Errno::result(res).map(drop)
}

libc_bitflags!(
/// Flags for [`open_tree()`].
pub struct OpenTreeFlags: c_uint {
/// Directly query the mount attached to the file descriptor.
AT_EMPTY_PATH as c_uint;
/// Do not trigger an automount target.
AT_NO_AUTOMOUNT as c_uint;
/// When cloning, also clone nested mount subtrees.
AT_RECURSIVE as c_uint;
/// If target is a symbolic link, do not dereference it.
AT_SYMLINK_NOFOLLOW as c_uint;
/// Clone the mount object.
OPEN_TREE_CLONE as c_uint;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this cast necessary, it seems that this constant is of type c_uint on both Linux and Android:

$ rg "OPEN_TREE_CLONE"
src/unix/linux_like/android/mod.rs
2174:pub const OPEN_TREE_CLONE: ::c_uint = 0x01;

src/unix/linux_like/linux/mod.rs
3328:pub const OPEN_TREE_CLONE: ::c_uint = 0x01;

libc-test/semver/linux.txt
1728:OPEN_TREE_CLONE

libc-test/semver/android.txt
1754:OPEN_TREE_CLONE

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To support both Linux and Android, we use the following alias, for more detail, see cfg-gates

#[cfg(linux_android)]

/// Set the close-on-exec flag for the returned file descriptor.
OPEN_TREE_CLOEXEC as c_uint;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same to this one:

$ rg "OPEN_TREE_CLOEXEC"
libc-test/semver/linux.txt
1727:OPEN_TREE_CLOEXEC

libc-test/semver/android.txt
1753:OPEN_TREE_CLOEXEC

src/unix/linux_like/linux/mod.rs
3329:pub const OPEN_TREE_CLOEXEC: ::c_uint = O_CLOEXEC as ::c_uint;

src/unix/linux_like/android/mod.rs
2175:pub const OPEN_TREE_CLOEXEC: ::c_uint = O_CLOEXEC as ::c_uint;

}
);

/// Find the mount object for the target path, and return it as a file-descriptor.
///
/// The returned FD behaves in the same way as those opened via `O_PATH`.
pub fn open_tree<Fd: AsFd, P: ?Sized + NixPath>(
dirfd: Option<Fd>,
pathname: &P,
flags: OpenTreeFlags,
) -> Result<OwnedFd> {
let res = pathname.with_nix_path(|cstr| unsafe {
let raw_dirfd = at_rawfd(dirfd.map(|v| v.as_fd().as_raw_fd()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at_rawfd() has been gated with:

#[cfg(any(
    all(feature = "fs", not(target_os = "redox")),
    all(feature = "process", any(target_os = "android", target_os = "linux"))
))]
pub(crate) fn at_rawfd(fd: Option<RawFd>) -> raw::c_int {
    fd.unwrap_or(libc::AT_FDCWD)
}

Given that open_tree() is under feature mount and for Linux and Android, we should probably gate at_rawfd with:

#[cfg(any(
    all(feature = "fs", not(target_os = "redox")),
    all(
        any(feature = "process", feature = "mount"),
        any(target_os = "android", target_os = "linux")
    )
))]
pub(crate) fn at_rawfd(fd: Option<RawFd>) -> raw::c_int {
    fd.unwrap_or(libc::AT_FDCWD)
}

libc::syscall(
libc::SYS_open_tree,
raw_dirfd,
cstr.as_ptr(),
flags.bits(),
)
})?;
Errno::result(res).map(|r| unsafe { OwnedFd::from_raw_fd(r as RawFd) })
}
31 changes: 29 additions & 2 deletions test/test_mount.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,13 @@ mod test_mount {
use std::io::{self, Read, Write};
use std::os::unix::fs::OpenOptionsExt;
use std::os::unix::fs::PermissionsExt;
use std::os::unix::io::{IntoRawFd, OwnedFd};
use std::process::{self, Command};

use libc::{EACCES, EROFS};

use nix::errno::Errno;
use nix::mount::{mount, umount, MsFlags};
use nix::mount::{mount, open_tree, umount, MsFlags, OpenTreeFlags};
use nix::sched::{unshare, CloneFlags};
use nix::sys::stat::{self, Mode};
use nix::unistd::getuid;
Expand Down Expand Up @@ -203,6 +204,31 @@ exit 23";
assert_eq!(buf, SCRIPT_CONTENTS);
}

pub(crate) fn test_open_tree() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: would like to change this to pub fn test_open_tree() so that it could be consistent with the other functions in this module :)

let tempdir = tempfile::tempdir().unwrap();
let abs_path = tempdir.path();
let res = open_tree(
Option::<OwnedFd>::None,
abs_path,
OpenTreeFlags::empty(),
);
let mount_fd = match res {
Ok(fd) => fd,
Err(e) if e == Errno::ENOSYS => {
let stderr = io::stderr();
let mut handle = stderr.lock();
writeln!(
handle,
"Detected kernel without `open_tree` syscall, skipping test."
)
.unwrap();
return;
}
Err(e) => panic!("{}", e),
};
nix::unistd::close(mount_fd.into_raw_fd()).unwrap()
}

pub fn setup_namespaces() {
// Hold on to the uid in the parent namespace.
let uid = getuid();
Expand Down Expand Up @@ -250,12 +276,13 @@ fn main() {
use test_mount::{
setup_namespaces, test_mount_bind, test_mount_noexec_disallows_exec,
test_mount_rdonly_disallows_write,
test_mount_tmpfs_without_flags_allows_rwx,
test_mount_tmpfs_without_flags_allows_rwx, test_open_tree,
};
skip_if_cirrus!("Fails for an unknown reason Cirrus CI. Bug #1351");
setup_namespaces();

run_tests!(
test_open_tree,
test_mount_tmpfs_without_flags_allows_rwx,
test_mount_rdonly_disallows_write,
test_mount_noexec_disallows_exec,
Expand Down