From a6abae2fbe4d963673095a36e2bb6d58c44242e0 Mon Sep 17 00:00:00 2001 From: Wang Siyuan Date: Tue, 20 Jan 2026 08:14:06 +0000 Subject: [PATCH] Add ptrace access mode checks --- kernel/src/fs/procfs/pid/task/maps.rs | 75 ++++++++++++++++- kernel/src/fs/procfs/pid/task/mem.rs | 82 +++++++++++++++++-- kernel/src/fs/procfs/template/file.rs | 25 +++++- kernel/src/process/clone.rs | 2 +- kernel/src/process/posix_thread/mod.rs | 5 +- kernel/src/process/posix_thread/ptrace.rs | 76 +++++++++++++++++ kernel/src/syscall/pidfd_getfd.rs | 29 +++---- .../src/syscall/gvisor/blocklists/proc_test | 2 +- 8 files changed, 262 insertions(+), 34 deletions(-) create mode 100644 kernel/src/process/posix_thread/ptrace.rs diff --git a/kernel/src/fs/procfs/pid/task/maps.rs b/kernel/src/fs/procfs/pid/task/maps.rs index 3f17a82f7..8b879da28 100644 --- a/kernel/src/fs/procfs/pid/task/maps.rs +++ b/kernel/src/fs/procfs/pid/task/maps.rs @@ -4,12 +4,21 @@ use aster_util::printer::VmPrinter; use super::TidDirOps; use crate::{ + events::IoEvents, fs::{ + inode_handle::FileIo, procfs::template::{FileOps, ProcFileBuilder}, - utils::{Inode, mkmod}, + utils::{AccessMode, Inode, InodeIo, StatusFlags, mkmod}, }, prelude::*, - process::{Process, posix_thread::AsPosixThread}, + process::{ + Process, + posix_thread::{ + AsPosixThread, + ptrace::{PtraceMode, check_may_access}, + }, + signal::{PollHandle, Pollable}, + }, vm::vmar::{VMAR_CAP_ADDR, VMAR_LOWEST_ADDR}, }; @@ -28,12 +37,44 @@ impl MapsFileOps { } impl FileOps for MapsFileOps { - fn read_at(&self, offset: usize, writer: &mut VmWriter) -> Result { + fn read_at(&self, _offset: usize, _writer: &mut VmWriter) -> Result { + unreachable!("should read via opened `MapsFileHandle`") + } + + fn write_at(&self, _offset: usize, _reader: &mut VmReader) -> Result { + unreachable!("should write via opened `MapsFileHandle`") + } + + fn open( + &self, + _access_mode: AccessMode, + _status_flags: StatusFlags, + ) -> Option>> { + let handle = check_may_access( + current_thread!().as_posix_thread().unwrap(), + self.0.main_thread().as_posix_thread().unwrap(), + PtraceMode::READ_FSCREDS, + ) + .map(|_| Box::new(MapsFileHandle(self.0.clone())) as Box); + + Some(handle) + } +} + +struct MapsFileHandle(Arc); + +impl InodeIo for MapsFileHandle { + fn read_at( + &self, + offset: usize, + writer: &mut VmWriter, + _status_flags: StatusFlags, + ) -> Result { let mut printer = VmPrinter::new_skip(writer, offset); let vmar_guard = self.0.lock_vmar(); let Some(vmar) = vmar_guard.as_ref() else { - return_errno_with_message!(Errno::ESRCH, "the process has exited"); + return Ok(0); }; let current = current_thread!(); @@ -47,4 +88,30 @@ impl FileOps for MapsFileOps { Ok(printer.bytes_written()) } + + fn write_at( + &self, + _offset: usize, + _reader: &mut VmReader, + _status_flags: StatusFlags, + ) -> Result { + return_errno_with_message!(Errno::EACCES, "`/proc/[pid]/maps` is read-only"); + } +} + +impl Pollable for MapsFileHandle { + fn poll(&self, mask: IoEvents, _poller: Option<&mut PollHandle>) -> IoEvents { + let events = IoEvents::IN | IoEvents::OUT; + events & mask + } +} + +impl FileIo for MapsFileHandle { + fn check_seekable(&self) -> Result<()> { + Ok(()) + } + + fn is_offset_aware(&self) -> bool { + true + } } diff --git a/kernel/src/fs/procfs/pid/task/mem.rs b/kernel/src/fs/procfs/pid/task/mem.rs index 237dd1ebf..4b6e5fc8f 100644 --- a/kernel/src/fs/procfs/pid/task/mem.rs +++ b/kernel/src/fs/procfs/pid/task/mem.rs @@ -2,12 +2,21 @@ use super::TidDirOps; use crate::{ + events::IoEvents, fs::{ + inode_handle::FileIo, procfs::template::{FileOps, ProcFileBuilder}, - utils::{Inode, mkmod}, + utils::{AccessMode, Inode, InodeIo, StatusFlags, mkmod}, }, prelude::*, - process::Process, + process::{ + Process, + posix_thread::{ + AsPosixThread, + ptrace::{PtraceMode, check_may_access}, + }, + signal::{PollHandle, Pollable}, + }, }; /// Represents the inode at `/proc/[pid]/task/[tid]/mem` (and also `/proc/[pid]/mem`). @@ -25,10 +34,49 @@ impl MemFileOps { } impl FileOps for MemFileOps { - fn read_at(&self, offset: usize, writer: &mut VmWriter) -> Result { + fn read_at(&self, _offset: usize, _writer: &mut VmWriter) -> Result { + unreachable!("should read via opened `MemFileHandle`") + } + + fn write_at(&self, _offset: usize, _reader: &mut VmReader) -> Result { + unreachable!("should write via opened `MemFileHandle`") + } + + fn open( + &self, + _access_mode: AccessMode, + _status_flags: StatusFlags, + ) -> Option>> { + if self.0.lock_vmar().as_ref().is_none() { + return Some(Err(Error::with_message( + Errno::EACCES, + "the process has exited", + ))); + } + + let handle = check_may_access( + current_thread!().as_posix_thread().unwrap(), + self.0.main_thread().as_posix_thread().unwrap(), + PtraceMode::ATTACH_FSCREDS, + ) + .map(|_| Box::new(MemFileHandle(self.0.clone())) as Box); + + Some(handle) + } +} + +struct MemFileHandle(Arc); + +impl InodeIo for MemFileHandle { + fn read_at( + &self, + offset: usize, + writer: &mut VmWriter, + _status_flags: StatusFlags, + ) -> Result { let vmar_guard = self.0.lock_vmar(); let Some(vmar) = vmar_guard.as_ref() else { - return_errno_with_message!(Errno::ESRCH, "the process has exited"); + return Ok(0); }; match vmar.read_remote(offset, writer) { Ok(bytes) => Ok(bytes), @@ -37,10 +85,15 @@ impl FileOps for MemFileOps { } } - fn write_at(&self, offset: usize, reader: &mut VmReader) -> Result { + fn write_at( + &self, + offset: usize, + reader: &mut VmReader, + _status_flags: StatusFlags, + ) -> Result { let vmar_guard = self.0.lock_vmar(); let Some(vmar) = vmar_guard.as_ref() else { - return_errno_with_message!(Errno::ESRCH, "the process has exited"); + return Ok(0); }; match vmar.write_remote(offset, reader) { Ok(bytes) => Ok(bytes), @@ -49,3 +102,20 @@ impl FileOps for MemFileOps { } } } + +impl Pollable for MemFileHandle { + fn poll(&self, mask: IoEvents, _poller: Option<&mut PollHandle>) -> IoEvents { + let events = IoEvents::IN | IoEvents::OUT; + events & mask + } +} + +impl FileIo for MemFileHandle { + fn check_seekable(&self) -> Result<()> { + Ok(()) + } + + fn is_offset_aware(&self) -> bool { + true + } +} diff --git a/kernel/src/fs/procfs/template/file.rs b/kernel/src/fs/procfs/template/file.rs index 509aa076f..e21e60aae 100644 --- a/kernel/src/fs/procfs/template/file.rs +++ b/kernel/src/fs/procfs/template/file.rs @@ -6,9 +6,12 @@ use inherit_methods_macro::inherit_methods; use super::{Common, ProcFs}; use crate::{ - fs::utils::{ - Extension, FileSystem, Inode, InodeIo, InodeMode, InodeType, Metadata, StatusFlags, - SymbolicLink, + fs::{ + inode_handle::FileIo, + utils::{ + AccessMode, Extension, FileSystem, Inode, InodeIo, InodeMode, InodeType, Metadata, + StatusFlags, SymbolicLink, + }, }, prelude::*, process::{Gid, Uid}, @@ -108,6 +111,14 @@ impl Inode for ProcFile { // Seeking regular files under `/proc` with `SEEK_END` will fail. None } + + fn open( + &self, + access_mode: AccessMode, + status_flags: StatusFlags, + ) -> Option>> { + self.inner.open(access_mode, status_flags) + } } pub trait FileOps: Sync + Send { @@ -116,4 +127,12 @@ pub trait FileOps: Sync + Send { fn write_at(&self, _offset: usize, _reader: &mut VmReader) -> Result { return_errno_with_message!(Errno::EPERM, "the file is not writable"); } + + fn open( + &self, + _access_mode: AccessMode, + _status_flags: StatusFlags, + ) -> Option>> { + None + } } diff --git a/kernel/src/process/clone.rs b/kernel/src/process/clone.rs index 1d9b6be43..3e3e3a5a7 100644 --- a/kernel/src/process/clone.rs +++ b/kernel/src/process/clone.rs @@ -391,7 +391,7 @@ fn clone_child_task( let mut thread_builder = PosixThreadBuilder::new(child_tid, thread_name, child_user_ctx, credentials) - .process(posix_thread.weak_process()) + .process(posix_thread.weak_process().clone()) .sig_mask(sig_mask) .file_table(child_file_table) .fs(child_fs) diff --git a/kernel/src/process/posix_thread/mod.rs b/kernel/src/process/posix_thread/mod.rs index f41598235..80c4e7c1d 100644 --- a/kernel/src/process/posix_thread/mod.rs +++ b/kernel/src/process/posix_thread/mod.rs @@ -30,6 +30,7 @@ mod exit; pub mod futex; mod name; mod posix_thread_ext; +pub mod ptrace; mod robust_list; mod thread_local; pub mod thread_table; @@ -97,8 +98,8 @@ impl PosixThread { self.process.upgrade().unwrap() } - pub fn weak_process(&self) -> Weak { - Weak::clone(&self.process) + pub fn weak_process(&self) -> &Weak { + &self.process } /// Returns the thread id diff --git a/kernel/src/process/posix_thread/ptrace.rs b/kernel/src/process/posix_thread/ptrace.rs new file mode 100644 index 000000000..934190342 --- /dev/null +++ b/kernel/src/process/posix_thread/ptrace.rs @@ -0,0 +1,76 @@ +// SPDX-License-Identifier: MPL-2.0 + +use bitflags::bitflags; + +use crate::{ + prelude::*, + process::{credentials::capabilities::CapSet, posix_thread::PosixThread}, +}; + +/// Checks whether the current `PosixThread` may access the given target `PosixThread` +// Reference: +pub fn check_may_access( + current_pthread: &PosixThread, + target_pthread: &PosixThread, + mode: PtraceMode, +) -> Result<()> { + if mode.contains(PtraceMode::FSCREDS) == mode.contains(PtraceMode::REALCREDS) { + return_errno_with_message!( + Errno::EPERM, + "should specify exactly one of FSCREDS and REALCREDS" + ); + } + + if Weak::ptr_eq( + current_pthread.weak_process(), + target_pthread.weak_process(), + ) { + return Ok(()); + } + + let cred = current_pthread.credentials(); + let (caller_uid, caller_gid) = if mode.contains(PtraceMode::FSCREDS) { + (cred.fsuid(), cred.fsgid()) + } else { + (cred.ruid(), cred.rgid()) + }; + + let tcred = target_pthread.credentials(); + let caller_is_same = caller_uid == tcred.euid() + && caller_uid == tcred.suid() + && caller_uid == tcred.ruid() + && caller_gid == tcred.egid() + && caller_gid == tcred.sgid() + && caller_gid == tcred.rgid(); + let caller_has_cap = target_pthread + .process() + .user_ns() + .lock() + .check_cap(CapSet::SYS_PTRACE, current_pthread) + .is_ok(); + + if !caller_is_same && !caller_has_cap { + return_errno_with_message!( + Errno::EPERM, + "the calling process does not have the required permissions" + ); + } + + // TODO: Add further security checks (e.g., YAMA LSM). + + Ok(()) +} + +bitflags! { + pub struct PtraceMode: u32 { + const READ = 0x01; + const ATTACH = 0x02; + const NOAUDIT = 0x04; + const FSCREDS = 0x08; + const REALCREDS = 0x10; + const READ_FSCREDS = Self::READ.bits() | Self::FSCREDS.bits(); + const READ_REALCREDS = Self::READ.bits() | Self::REALCREDS.bits(); + const ATTACH_FSCREDS = Self::ATTACH.bits() | Self::FSCREDS.bits(); + const ATTACH_REALCREDS = Self::ATTACH.bits() | Self::REALCREDS.bits(); + } +} diff --git a/kernel/src/syscall/pidfd_getfd.rs b/kernel/src/syscall/pidfd_getfd.rs index b0cfdf952..323e5d740 100644 --- a/kernel/src/syscall/pidfd_getfd.rs +++ b/kernel/src/syscall/pidfd_getfd.rs @@ -3,7 +3,13 @@ use crate::{ fs::file_table::{FdFlags, FileDesc, get_file_fast}, prelude::*, - process::{PidFile, credentials::capabilities::CapSet, posix_thread::AsPosixThread}, + process::{ + PidFile, + posix_thread::{ + AsPosixThread, + ptrace::{PtraceMode, check_may_access}, + }, + }, syscall::SyscallReturn, }; @@ -32,22 +38,11 @@ pub fn sys_pidfd_getfd( .process_opt() .ok_or_else(|| Error::with_message(Errno::ESRCH, "the target process has been reaped"))?; - // The calling process should have PTRACE_MODE_ATTACH_REALCREDS permissions (see ptrace(2)) - // over the process referred to by `pidfd`. - // Currently, this is implemented as requiring the calling process to have the - // CAP_SYS_PTRACE capability, which is stricter. - // TODO: Implement appropriate PTRACE_MODE_ATTACH_REALCREDS permission check. - if process - .user_ns() - .lock() - .check_cap(CapSet::SYS_PTRACE, ctx.posix_thread) - .is_err() - { - return_errno_with_message!( - Errno::EPERM, - "the calling process does not have the required permissions" - ); - } + check_may_access( + ctx.posix_thread, + process.main_thread().as_posix_thread().unwrap(), + PtraceMode::ATTACH_REALCREDS, + )?; let main_thread = process.main_thread(); diff --git a/test/initramfs/src/syscall/gvisor/blocklists/proc_test b/test/initramfs/src/syscall/gvisor/blocklists/proc_test index 0e072e159..8c5465144 100644 --- a/test/initramfs/src/syscall/gvisor/blocklists/proc_test +++ b/test/initramfs/src/syscall/gvisor/blocklists/proc_test @@ -12,7 +12,7 @@ ProcPid.AccessDeny ProcPidCwd.Subprocess ProcPidRoot.Subprocess ProcPidEnviron.MatchesEnviron -ProcPidMem.AfterExit +# TODO: support `ptrace`. ProcPidMem.DifferentUserAttached ProcPidFile.SubprocessRunning ProcPidFile.SubprocessZombie