Skip to content

Commit

Permalink
Fix procfs on /proc/self with user namespaces (#859)
Browse files Browse the repository at this point in the history
* Fix procfs on /proc/self with user namespaces

Use `readlink` on the /proc/self symlink instead of `getpid`, so
that we get the pid relative to the procfs's pid namespace rather
than the process' pid namespace.

Fixes #854.

* Avoid using `Vec`.

* Use `RawDir` to avoid allocationo.
  • Loading branch information
sunfishcode authored Sep 29, 2023
1 parent 5f0db52 commit 69319a3
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 45 deletions.
12 changes: 10 additions & 2 deletions src/backend/libc/fs/syscalls.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
//! libc syscalls supporting `rustix::fs`.
use crate::backend::c;
#[cfg(any(apple, linux_kernel, feature = "alloc"))]
#[cfg(any(
apple,
linux_kernel,
feature = "alloc",
all(linux_kernel, feature = "procfs")
))]
use crate::backend::conv::ret_usize;
use crate::backend::conv::{borrowed_fd, c_str, ret, ret_c_int, ret_off_t, ret_owned_fd};
use crate::fd::{BorrowedFd, OwnedFd};
Expand Down Expand Up @@ -258,7 +263,10 @@ pub(crate) fn readlink(path: &CStr, buf: &mut [u8]) -> io::Result<usize> {
}
}

#[cfg(all(feature = "alloc", not(target_os = "redox")))]
#[cfg(all(
any(feature = "alloc", all(linux_kernel, feature = "procfs")),
not(target_os = "redox")
))]
#[inline]
pub(crate) fn readlinkat(
dirfd: BorrowedFd<'_>,
Expand Down
2 changes: 1 addition & 1 deletion src/backend/linux_raw/fs/syscalls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -951,7 +951,7 @@ pub(crate) fn readlink(path: &CStr, buf: &mut [u8]) -> io::Result<usize> {
}
}

#[cfg(feature = "alloc")]
#[cfg(any(feature = "alloc", all(linux_kernel, feature = "procfs")))]
#[inline]
pub(crate) fn readlinkat(
dirfd: BorrowedFd<'_>,
Expand Down
9 changes: 6 additions & 3 deletions src/fs/raw_dir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,10 +43,11 @@ impl<'buf, Fd: AsFd> RawDir<'buf, Fd> {
/// ```
/// # use std::mem::MaybeUninit;
/// # use rustix::fs::{CWD, Mode, OFlags, openat, RawDir};
/// # use rustix::cstr;
///
/// let fd = openat(
/// CWD,
/// ".",
/// cstr!("."),
/// OFlags::RDONLY | OFlags::DIRECTORY | OFlags::CLOEXEC,
/// Mode::empty(),
/// )
Expand All @@ -65,10 +66,11 @@ impl<'buf, Fd: AsFd> RawDir<'buf, Fd> {
/// ```
/// # use std::mem::MaybeUninit;
/// # use rustix::fs::{CWD, Mode, OFlags, openat, RawDir};
/// # use rustix::cstr;
///
/// let fd = openat(
/// CWD,
/// ".",
/// cstr!("."),
/// OFlags::RDONLY | OFlags::DIRECTORY | OFlags::CLOEXEC,
/// Mode::empty(),
/// )
Expand All @@ -92,10 +94,11 @@ impl<'buf, Fd: AsFd> RawDir<'buf, Fd> {
/// # use std::mem::MaybeUninit;
/// # use rustix::fs::{CWD, Mode, OFlags, openat, RawDir};
/// # use rustix::io::Errno;
/// # use rustix::cstr;
///
/// let fd = openat(
/// CWD,
/// ".",
/// cstr!("."),
/// OFlags::RDONLY | OFlags::DIRECTORY | OFlags::CLOEXEC,
/// Mode::empty(),
/// )
Expand Down
114 changes: 75 additions & 39 deletions src/procfs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,20 +18,19 @@
//! namespace. So with the checking here, they may fail, but they won't be able
//! to succeed with bogus results.
use crate::backend::pid::syscalls::getpid;
use crate::fd::{AsFd, BorrowedFd, OwnedFd};
use crate::ffi::CStr;
use crate::fs::{
fstat, fstatfs, major, openat, renameat, FileType, FsWord, Mode, OFlags, Stat, CWD,
fstat, fstatfs, major, openat, renameat, FileType, FsWord, Mode, OFlags, RawDir, Stat, CWD,
PROC_SUPER_MAGIC,
};
use crate::io;
use crate::path::DecInt;
#[cfg(feature = "rustc-dep-of-std")]
use core::lazy::OnceCell;
use core::mem::MaybeUninit;
#[cfg(not(feature = "rustc-dep-of-std"))]
use once_cell::sync::OnceCell;
#[cfg(feature = "alloc")]
use {crate::ffi::CStr, crate::fs::Dir};

/// Linux's procfs always uses inode 1 for its root directory.
const PROC_ROOT_INO: u64 = 1;
Expand All @@ -42,8 +41,8 @@ enum Kind {
Proc,
Pid,
Fd,
#[cfg(feature = "alloc")]
File,
Symlink,
}

/// Check a subdirectory of "/proc" for anomalies.
Expand All @@ -69,16 +68,23 @@ fn check_proc_entry_with_stat(
match kind {
Kind::Proc => check_proc_root(entry, &entry_stat)?,
Kind::Pid | Kind::Fd => check_proc_subdir(entry, &entry_stat, proc_stat)?,
#[cfg(feature = "alloc")]
Kind::File => check_proc_file(&entry_stat, proc_stat)?,
Kind::Symlink => check_proc_symlink(&entry_stat, proc_stat)?,
}

// "/proc" directories are typically mounted r-xr-xr-x.
// "/proc/self/fd" is r-x------. Allow them to have fewer permissions, but
// not more.
let expected_mode = if let Kind::Fd = kind { 0o500 } else { 0o555 };
if entry_stat.st_mode & 0o777 & !expected_mode != 0 {
return Err(io::Errno::NOTSUP);
match kind {
Kind::Symlink => {
// On Linux, symlinks don't have their own permissions.
}
_ => {
let expected_mode = if let Kind::Fd = kind { 0o500 } else { 0o555 };
if entry_stat.st_mode & 0o777 & !expected_mode != 0 {
return Err(io::Errno::NOTSUP);
}
}
}

match kind {
Expand All @@ -97,14 +103,20 @@ fn check_proc_entry_with_stat(
return Err(io::Errno::NOTSUP);
}
}
#[cfg(feature = "alloc")]
Kind::File => {
// Check that files in procfs don't have extraneous hard links to
// them (which might indicate hard links to other things).
if entry_stat.st_nlink != 1 {
return Err(io::Errno::NOTSUP);
}
}
Kind::Symlink => {
// Check that symlinks in procfs don't have extraneous hard links
// to them (which might indicate hard links to other things).
if entry_stat.st_nlink != 1 {
return Err(io::Errno::NOTSUP);
}
}
}

Ok(entry_stat)
Expand Down Expand Up @@ -153,7 +165,6 @@ fn check_proc_subdir(
Ok(())
}

#[cfg(feature = "alloc")]
fn check_proc_file(stat: &Stat, proc_stat: Option<&Stat>) -> io::Result<()> {
// Check that we have a regular file.
if FileType::from_raw_mode(stat.st_mode) != FileType::RegularFile {
Expand All @@ -165,6 +176,17 @@ fn check_proc_file(stat: &Stat, proc_stat: Option<&Stat>) -> io::Result<()> {
Ok(())
}

fn check_proc_symlink(stat: &Stat, proc_stat: Option<&Stat>) -> io::Result<()> {
// Check that we have a symbolic link.
if FileType::from_raw_mode(stat.st_mode) != FileType::Symlink {
return Err(io::Errno::NOTSUP);
}

check_proc_nonroot(stat, proc_stat)?;

Ok(())
}

fn check_proc_nonroot(stat: &Stat, proc_stat: Option<&Stat>) -> io::Result<()> {
// Check that we haven't been linked back to the root of "/proc".
if stat.st_ino == PROC_ROOT_INO {
Expand Down Expand Up @@ -245,6 +267,7 @@ fn proc() -> io::Result<(BorrowedFd<'static>, &'static Stat)> {
/// - [Linux]
///
/// [Linux]: https://man7.org/linux/man-pages/man5/proc.5.html
#[allow(unsafe_code)]
fn proc_self() -> io::Result<(BorrowedFd<'static>, &'static Stat)> {
static PROC_SELF: StaticFd = StaticFd::new();

Expand All @@ -253,11 +276,21 @@ fn proc_self() -> io::Result<(BorrowedFd<'static>, &'static Stat)> {
.get_or_try_init(|| {
let (proc, proc_stat) = proc()?;

let pid = getpid();
// `getpid` would return our pid in our own pid namespace, so
// instead use `readlink` on the `self` symlink to learn our pid in
// the procfs namespace.
let self_symlink = open_and_check_file(proc, proc_stat, cstr!("self"), Kind::Symlink)?;
let mut buf = [MaybeUninit::<u8>::uninit(); 20];
let len = crate::backend::fs::syscalls::readlinkat(
self_symlink.as_fd(),
cstr!(""),
&mut buf,
)?;
let pid: &[u8] = unsafe { core::mem::transmute(&buf[..len]) };

// Open "/proc/self". Use our pid to compute the name rather than
// literally using "self", as "self" is a symlink.
let proc_self = proc_opendirat(proc, DecInt::new(pid.as_raw_nonzero().get()))?;
let proc_self = proc_opendirat(proc, pid)?;
let proc_self_stat = check_proc_entry(Kind::Pid, proc_self.as_fd(), Some(proc_stat))
.map_err(|_err| io::Errno::NOTSUP)?;

Expand Down Expand Up @@ -314,7 +347,6 @@ fn new_static_fd(fd: OwnedFd, stat: Stat) -> (OwnedFd, Stat) {
/// - [Linux]
///
/// [Linux]: https://man7.org/linux/man-pages/man5/proc.5.html
#[cfg(feature = "alloc")]
fn proc_self_fdinfo() -> io::Result<(BorrowedFd<'static>, &'static Stat)> {
static PROC_SELF_FDINFO: StaticFd = StaticFd::new();

Expand Down Expand Up @@ -344,18 +376,21 @@ fn proc_self_fdinfo() -> io::Result<(BorrowedFd<'static>, &'static Stat)> {
/// - [Linux]
///
/// [Linux]: https://man7.org/linux/man-pages/man5/proc.5.html
#[cfg(feature = "alloc")]
#[inline]
#[cfg_attr(doc_cfg, doc(cfg(feature = "procfs")))]
pub fn proc_self_fdinfo_fd<Fd: AsFd>(fd: Fd) -> io::Result<OwnedFd> {
_proc_self_fdinfo(fd.as_fd())
}

#[cfg(feature = "alloc")]
fn _proc_self_fdinfo(fd: BorrowedFd<'_>) -> io::Result<OwnedFd> {
let (proc_self_fdinfo, proc_self_fdinfo_stat) = proc_self_fdinfo()?;
let fd_str = DecInt::from_fd(fd);
open_and_check_file(proc_self_fdinfo, proc_self_fdinfo_stat, fd_str.as_c_str())
open_and_check_file(
proc_self_fdinfo,
proc_self_fdinfo_stat,
fd_str.as_c_str(),
Kind::File,
)
}

/// Returns a handle to a Linux `/proc/self/pagemap` file.
Expand All @@ -369,7 +404,6 @@ fn _proc_self_fdinfo(fd: BorrowedFd<'_>) -> io::Result<OwnedFd> {
///
/// [Linux]: https://man7.org/linux/man-pages/man5/proc.5.html
/// [Linux pagemap]: https://www.kernel.org/doc/Documentation/vm/pagemap.txt
#[cfg(feature = "alloc")]
#[inline]
#[cfg_attr(doc_cfg, doc(cfg(feature = "procfs")))]
pub fn proc_self_pagemap() -> io::Result<OwnedFd> {
Expand All @@ -385,7 +419,6 @@ pub fn proc_self_pagemap() -> io::Result<OwnedFd> {
/// - [Linux]
///
/// [Linux]: https://man7.org/linux/man-pages/man5/proc.5.html
#[cfg(feature = "alloc")]
#[inline]
#[cfg_attr(doc_cfg, doc(cfg(feature = "procfs")))]
pub fn proc_self_maps() -> io::Result<OwnedFd> {
Expand All @@ -401,23 +434,25 @@ pub fn proc_self_maps() -> io::Result<OwnedFd> {
/// - [Linux]
///
/// [Linux]: https://man7.org/linux/man-pages/man5/proc.5.html
#[cfg(feature = "alloc")]
#[inline]
#[cfg_attr(doc_cfg, doc(cfg(feature = "procfs")))]
pub fn proc_self_status() -> io::Result<OwnedFd> {
proc_self_file(cstr!("status"))
}

/// Open a file under `/proc/self`.
#[cfg(feature = "alloc")]
fn proc_self_file(name: &CStr) -> io::Result<OwnedFd> {
let (proc_self, proc_self_stat) = proc_self()?;
open_and_check_file(proc_self, proc_self_stat, name)
open_and_check_file(proc_self, proc_self_stat, name, Kind::File)
}

/// Open a procfs file within in `dir` and check it for bind mounts.
#[cfg(feature = "alloc")]
fn open_and_check_file(dir: BorrowedFd<'_>, dir_stat: &Stat, name: &CStr) -> io::Result<OwnedFd> {
fn open_and_check_file(
dir: BorrowedFd<'_>,
dir_stat: &Stat,
name: &CStr,
kind: Kind,
) -> io::Result<OwnedFd> {
let (_, proc_stat) = proc()?;

// Don't use `NOATIME`, because it [requires us to own the file], and when
Expand All @@ -426,7 +461,11 @@ fn open_and_check_file(dir: BorrowedFd<'_>, dir_stat: &Stat, name: &CStr) -> io:
//
// [requires us to own the file]: https://man7.org/linux/man-pages/man2/openat.2.html
// [to root:root]: https://man7.org/linux/man-pages/man5/proc.5.html
let oflags = OFlags::RDONLY | OFlags::CLOEXEC | OFlags::NOFOLLOW | OFlags::NOCTTY;
let mut oflags = OFlags::RDONLY | OFlags::CLOEXEC | OFlags::NOFOLLOW | OFlags::NOCTTY;
if let Kind::Symlink = kind {
// Open symlinks with `O_PATH`.
oflags |= OFlags::PATH;
}
let file = openat(dir, name, oflags, Mode::empty()).map_err(|_err| io::Errno::NOTSUP)?;
let file_stat = fstat(&file)?;

Expand All @@ -436,32 +475,29 @@ fn open_and_check_file(dir: BorrowedFd<'_>, dir_stat: &Stat, name: &CStr) -> io:
// we just opened. If we can't find it, there could be a file bind mount on
// top of the file we want.
//
// As we scan, we also check for ".", to make sure it's the same directory
// as our original directory, to detect mount points, since
// `Dir::read_from` reopens ".".
//
// TODO: With Linux 5.8 we might be able to use `statx` and
// `STATX_ATTR_MOUNT_ROOT` to detect mountpoints directly instead of doing
// this scanning.
let dir = Dir::read_from(dir).map_err(|_err| io::Errno::NOTSUP)?;

// Confirm that we got the same inode.
let dot_stat = dir.stat().map_err(|_err| io::Errno::NOTSUP)?;
if (dot_stat.st_dev, dot_stat.st_ino) != (dir_stat.st_dev, dir_stat.st_ino) {
return Err(io::Errno::NOTSUP);
}
let expected_type = match kind {
Kind::File => FileType::RegularFile,
Kind::Symlink => FileType::Symlink,
_ => unreachable!(),
};

let mut found_file = false;
let mut found_dot = false;
for entry in dir {

let mut buf = [MaybeUninit::uninit(); 2048];
let mut iter = RawDir::new(dir, &mut buf);
while let Some(entry) = iter.next() {
let entry = entry.map_err(|_err| io::Errno::NOTSUP)?;
if entry.ino() == file_stat.st_ino
&& entry.file_type() == FileType::RegularFile
&& entry.file_type() == expected_type
&& entry.file_name() == name
{
// We found the file. Proceed to check the file handle.
let _ =
check_proc_entry_with_stat(Kind::File, file.as_fd(), file_stat, Some(proc_stat))?;
let _ = check_proc_entry_with_stat(kind, file.as_fd(), file_stat, Some(proc_stat))?;

found_file = true;
} else if entry.ino() == dir_stat.st_ino
Expand Down

0 comments on commit 69319a3

Please sign in to comment.