From 580e98b9610b0e782683233f10e553148d114faa Mon Sep 17 00:00:00 2001 From: longjin Date: Tue, 15 Sep 2026 10:43:04 +0000 Subject: [PATCH 1/7] fix(procfs): freeze per-fd snapshots, re-parent every thread, resolve /proc/ by tid /proc exposed three behaviours that disagree with Linux 6.6. 1. Every read_at() re-rendered the record and then sliced it with the file position, so a second read() could hand back the tail of a longer render instead of staying at EOF. Serve the seq-style files the way seq_read_iter() does: render once per fd, replay that snapshot, and re-render only on rewind or when the position no longer matches the continuation point. A failed render drops the continuation point, as a failed traverse() resets the buffer. Files Linux serves through a plain ->read (oom_score_adj, /proc//cmdline, sys/*) keep streaming on purpose. 2. Re-parenting rewrote only the group leader, so /proc//task//status reported a different Ppid than /proc//status for the same thread group. Walk the whole group the way forget_original_parent() does. 3. /proc/ resolved through the TGID link only, so a non-leader tid had no directory. Resolve lookups through the PID link (proc_pid_lookup()) while directory listing keeps using the TGID link (next_tgid()), and drop non-leader entries from the per-directory cache so the listing cannot leak a tid that lookup created. Add normal/procfs_task_semantics to the dunitest whitelist. Five of its twelve cases fail on the pre-fix kernel; the rest guard the directions that must not change. Signed-off-by: longjin --- kernel/src/filesystem/procfs/cmdline.rs | 11 +- kernel/src/filesystem/procfs/cpuinfo.rs | 34 +- kernel/src/filesystem/procfs/loadavg.rs | 10 +- kernel/src/filesystem/procfs/meminfo.rs | 10 +- kernel/src/filesystem/procfs/mod.rs | 6 + .../procfs/mount/inode/pid_mount.rs | 28 +- kernel/src/filesystem/procfs/mount/mod.rs | 2 +- kernel/src/filesystem/procfs/mount/render.rs | 48 +- kernel/src/filesystem/procfs/net/arp.rs | 11 +- kernel/src/filesystem/procfs/net/protocols.rs | 11 +- kernel/src/filesystem/procfs/pid/cgroup.rs | 8 +- kernel/src/filesystem/procfs/pid/id_map.rs | 89 +- kernel/src/filesystem/procfs/pid/limits.rs | 10 +- kernel/src/filesystem/procfs/pid/maps.rs | 11 +- kernel/src/filesystem/procfs/pid/mod.rs | 23 +- .../filesystem/procfs/pid/oom_score_adj.rs | 18 +- kernel/src/filesystem/procfs/pid/stat.rs | 31 +- kernel/src/filesystem/procfs/pid/statm.rs | 55 +- kernel/src/filesystem/procfs/pid/status.rs | 14 +- kernel/src/filesystem/procfs/root.rs | 23 +- kernel/src/filesystem/procfs/stat.rs | 11 +- kernel/src/filesystem/procfs/utils.rs | 56 ++ kernel/src/filesystem/procfs/version.rs | 10 +- .../filesystem/procfs/version_signature.rs | 10 +- kernel/src/filesystem/procfs/vmstat.rs | 11 +- kernel/src/process/task.rs | 59 +- .../suites/normal/proc_task_status.cc | 17 +- .../suites/normal/procfs_task_semantics.cc | 926 ++++++++++++++++++ user/apps/tests/dunitest/whitelist.txt | 1 + 29 files changed, 1330 insertions(+), 224 deletions(-) create mode 100644 user/apps/tests/dunitest/suites/normal/procfs_task_semantics.cc diff --git a/kernel/src/filesystem/procfs/cmdline.rs b/kernel/src/filesystem/procfs/cmdline.rs index 6dfa45f81e..b63d02f743 100644 --- a/kernel/src/filesystem/procfs/cmdline.rs +++ b/kernel/src/filesystem/procfs/cmdline.rs @@ -9,7 +9,7 @@ use crate::{ filesystem::{ procfs::{ template::{Builder, FileOps, ProcFileBuilder}, - utils::proc_read, + utils::proc_read_snapshot, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }, @@ -53,10 +53,11 @@ impl FileOps for CmdlineFileOps { offset: usize, len: usize, buf: &mut [u8], - _data: MutexGuard, + mut data: MutexGuard, ) -> Result { - let content = Self::generate_cmdline_content(); - - proc_read(offset, len, buf, &content) + // `single_open()`: one fd sees one kernel command line record. + proc_read_snapshot(offset, len, buf, &mut data, || { + Ok(Self::generate_cmdline_content()) + }) } } diff --git a/kernel/src/filesystem/procfs/cpuinfo.rs b/kernel/src/filesystem/procfs/cpuinfo.rs index 0e44de186b..8c4210f755 100644 --- a/kernel/src/filesystem/procfs/cpuinfo.rs +++ b/kernel/src/filesystem/procfs/cpuinfo.rs @@ -5,7 +5,7 @@ use crate::{ filesystem::{ procfs::{ template::{Builder, FileOps, ProcFileBuilder}, - utils::proc_read, + utils::proc_read_snapshot, }, vfs::{IndexNode, InodeMode}, }, @@ -34,22 +34,26 @@ impl FileOps for CpuInfoFileOps { offset: usize, len: usize, buf: &mut [u8], - _: crate::libs::mutex::MutexGuard, + mut data: crate::libs::mutex::MutexGuard, ) -> Result { - let mut data: Vec = vec![]; - let cpu_manager = smp_cpu_manager(); - - // 遍历所有present的CPU - for cpu_id in cpu_manager.present_cpus().iter_cpu() { - // 生成每个 CPU 的信息 - let cpu_info = generate_cpu_info(cpu_id); - data.extend_from_slice(cpu_info.as_bytes()); - - // 在每个CPU信息之间添加空行分隔 - data.push(b'\n'); - } + // `seq_file` (Linux `seq_open(&cpuinfo_op)`, `fs/proc/cpuinfo.c`): one + // fd sees one rendered record. + proc_read_snapshot(offset, len, buf, &mut data, || { + let mut content: Vec = vec![]; + let cpu_manager = smp_cpu_manager(); + + // Walk every present CPU. + for cpu_id in cpu_manager.present_cpus().iter_cpu() { + // Render this CPU's record. + let cpu_info = generate_cpu_info(cpu_id); + content.extend_from_slice(cpu_info.as_bytes()); + + // Blank line between CPU records, as Linux does. + content.push(b'\n'); + } - proc_read(offset, len, buf, &data) + Ok(content) + }) } } diff --git a/kernel/src/filesystem/procfs/loadavg.rs b/kernel/src/filesystem/procfs/loadavg.rs index 398bdfd6b4..30ec0ac85c 100644 --- a/kernel/src/filesystem/procfs/loadavg.rs +++ b/kernel/src/filesystem/procfs/loadavg.rs @@ -3,7 +3,7 @@ use crate::{ filesystem::{ procfs::{ template::{Builder, FileOps, ProcFileBuilder}, - utils::{proc_read, trim_string}, + utils::{proc_read_snapshot, trim_string}, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }, @@ -71,9 +71,11 @@ impl FileOps for LoadavgFileOps { offset: usize, len: usize, buf: &mut [u8], - _data: MutexGuard, + mut data: MutexGuard, ) -> Result { - let content = Self::generate_loadavg_content(); - proc_read(offset, len, buf, &content) + // `single_open()`: one fd sees one load-average record. + proc_read_snapshot(offset, len, buf, &mut data, || { + Ok(Self::generate_loadavg_content()) + }) } } diff --git a/kernel/src/filesystem/procfs/meminfo.rs b/kernel/src/filesystem/procfs/meminfo.rs index 224457456d..598e9e5732 100644 --- a/kernel/src/filesystem/procfs/meminfo.rs +++ b/kernel/src/filesystem/procfs/meminfo.rs @@ -8,7 +8,7 @@ use crate::{ filesystem::{ procfs::{ template::{Builder, FileOps, ProcFileBuilder}, - utils::{proc_read, trim_string}, + utils::{proc_read_snapshot, trim_string}, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }, @@ -107,9 +107,11 @@ impl FileOps for MeminfoFileOps { offset: usize, len: usize, buf: &mut [u8], - _data: MutexGuard, + mut data: MutexGuard, ) -> Result { - let content = Self::generate_meminfo_content(); - proc_read(offset, len, buf, &content) + // `single_open()`: one fd sees one meminfo record. + proc_read_snapshot(offset, len, buf, &mut data, || { + Ok(Self::generate_meminfo_content()) + }) } } diff --git a/kernel/src/filesystem/procfs/mod.rs b/kernel/src/filesystem/procfs/mod.rs index 017b7b4b52..298ce843c6 100644 --- a/kernel/src/filesystem/procfs/mod.rs +++ b/kernel/src/filesystem/procfs/mod.rs @@ -53,6 +53,11 @@ pub struct ProcfsFilePrivateData { pub data: Vec, pub open_cred: Arc, pub pinned_vm: Option>, + /// Continuation position for seq-style files (mirrors Linux `seq_file::m->read_pos`). + /// + /// `None` means this fd has not rendered yet; `Some(p)` means the snapshot is + /// ready and the next read continues at `p`. See `utils::proc_read_snapshot()`. + pub read_pos: Option, } impl ProcfsFilePrivateData { @@ -61,6 +66,7 @@ impl ProcfsFilePrivateData { data: Vec::new(), open_cred: ProcessManager::current_pcb().cred(), pinned_vm: None, + read_pos: None, } } } diff --git a/kernel/src/filesystem/procfs/mount/inode/pid_mount.rs b/kernel/src/filesystem/procfs/mount/inode/pid_mount.rs index d3aed3214d..81f56a5028 100644 --- a/kernel/src/filesystem/procfs/mount/inode/pid_mount.rs +++ b/kernel/src/filesystem/procfs/mount/inode/pid_mount.rs @@ -2,9 +2,10 @@ use core::fmt::Debug; use crate::filesystem::{ procfs::{ - mount::{open_mount_file_for_target, read_cached_mount_file, ProcMountRenderKind}, + mount::{render_mount_file_for_task, ProcMountRenderKind}, pid::ProcPidTarget, template::{Builder, FileOps, ProcFileBuilder}, + utils::proc_read_snapshot, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }; @@ -41,8 +42,12 @@ impl FileOps for MountProcFileOps { self.target.owner_uid_gid() } - fn open(&self, data: &mut MutexGuard) -> Result<(), SystemError> { - open_mount_file_for_target(&self.target, self.kind, data) + fn open(&self, _data: &mut MutexGuard) -> Result<(), SystemError> { + // Linux `mounts_open_common()` resolves the target with `get_proc_task()` + // at open time and fails with `ESRCH` when it is already gone. The record + // itself is rendered on the first read, like any other `seq_file`. + self.target.thread_group_leader().ok_or(SystemError::ESRCH)?; + Ok(()) } fn read_at( @@ -50,11 +55,18 @@ impl FileOps for MountProcFileOps { offset: usize, len: usize, buf: &mut [u8], - data: MutexGuard, + mut data: MutexGuard, ) -> Result { - self.target - .thread_group_leader() - .ok_or(SystemError::ESRCH)?; - read_cached_mount_file(offset, len, buf, data) + // The target is resolved by the renderer, never on a continuation read. + // `seq_read_iter()` does not re-enter a handler while its buffer still + // holds data, so a reader that already took the first chunk keeps + // draining this fd's snapshot even after the thread group is gone. + proc_read_snapshot(offset, len, buf, &mut data, || { + let task = self + .target + .thread_group_leader() + .ok_or(SystemError::ESRCH)?; + render_mount_file_for_task(&task, self.kind) + }) } } diff --git a/kernel/src/filesystem/procfs/mount/mod.rs b/kernel/src/filesystem/procfs/mount/mod.rs index 308857483d..2dbd678d9e 100644 --- a/kernel/src/filesystem/procfs/mount/mod.rs +++ b/kernel/src/filesystem/procfs/mount/mod.rs @@ -8,4 +8,4 @@ pub(crate) mod format; pub(crate) mod inode; mod render; -pub(crate) use render::{open_mount_file_for_target, read_cached_mount_file, ProcMountRenderKind}; +pub(crate) use render::{render_mount_file_for_task, ProcMountRenderKind}; diff --git a/kernel/src/filesystem/procfs/mount/render.rs b/kernel/src/filesystem/procfs/mount/render.rs index 3ef1cbcfdd..40fa8af1fb 100644 --- a/kernel/src/filesystem/procfs/mount/render.rs +++ b/kernel/src/filesystem/procfs/mount/render.rs @@ -3,11 +3,7 @@ use alloc::{string::String, sync::Arc, vec::Vec}; use system_error::SystemError; use crate::{ - filesystem::{ - procfs::{pid::ProcPidTarget, utils::proc_read}, - vfs::{mount::with_topology_snapshot, FilePrivateData}, - }, - libs::mutex::MutexGuard, + filesystem::vfs::mount::with_topology_snapshot, process::ProcessControlBlock, }; @@ -24,41 +20,13 @@ pub(crate) enum ProcMountRenderKind { MountStats, } -pub(crate) fn open_mount_file_for_target( - target: &ProcPidTarget, - kind: ProcMountRenderKind, - data: &mut MutexGuard, -) -> Result<(), SystemError> { - let task = target.thread_group_leader().ok_or(SystemError::ESRCH)?; - open_mount_file_for_task(&task, kind, data) -} - -fn open_mount_file_for_task( - task: &Arc, - kind: ProcMountRenderKind, - data: &mut MutexGuard, -) -> Result<(), SystemError> { - let rendered = render_mount_file_for_task(task, kind)?; - let FilePrivateData::Procfs(pdata) = &mut **data else { - return Err(SystemError::EIO); - }; - pdata.data = rendered; - Ok(()) -} - -pub(crate) fn read_cached_mount_file( - offset: usize, - len: usize, - buf: &mut [u8], - data: MutexGuard, -) -> Result { - match &*data { - FilePrivateData::Procfs(pdata) => proc_read(offset, len, buf, &pdata.data), - _ => Err(SystemError::EINVAL), - } -} - -fn render_mount_file_for_task( +/// Render one mount-family record (`mounts` / `mountinfo` / `mountstats`) for +/// `target`. +/// +/// Linux serves these through `seq_open_private()` (`fs/proc_namespace.c`): the +/// record is produced by the reader, not at open time, so a file that is opened +/// and read much later shows the topology of the moment it is read. +pub(crate) fn render_mount_file_for_task( target: &Arc, kind: ProcMountRenderKind, ) -> Result, SystemError> { diff --git a/kernel/src/filesystem/procfs/net/arp.rs b/kernel/src/filesystem/procfs/net/arp.rs index c6bc9dadf6..b2aadb6b29 100644 --- a/kernel/src/filesystem/procfs/net/arp.rs +++ b/kernel/src/filesystem/procfs/net/arp.rs @@ -8,7 +8,7 @@ use crate::filesystem::{ procfs::{ template::{Builder, FileOps, ProcFileBuilder}, - utils::proc_read, + utils::proc_read_snapshot, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }; @@ -81,9 +81,12 @@ impl FileOps for ArpFileOps { offset: usize, len: usize, buf: &mut [u8], - _data: MutexGuard, + mut data: MutexGuard, ) -> Result { - let content = Self::generate_arp_content(); - proc_read(offset, len, buf, &content) + // `seq_file` (Linux `proc_create_net(&arp_seq_ops)`, `net/ipv4/arp.c`): one + // fd sees one rendered record. + proc_read_snapshot(offset, len, buf, &mut data, || { + Ok(Self::generate_arp_content()) + }) } } diff --git a/kernel/src/filesystem/procfs/net/protocols.rs b/kernel/src/filesystem/procfs/net/protocols.rs index 3b22484ce7..59be58f33a 100644 --- a/kernel/src/filesystem/procfs/net/protocols.rs +++ b/kernel/src/filesystem/procfs/net/protocols.rs @@ -8,7 +8,7 @@ use crate::filesystem::{ procfs::{ template::{Builder, FileOps, ProcFileBuilder}, - utils::proc_read, + utils::proc_read_snapshot, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }; @@ -155,9 +155,12 @@ impl FileOps for ProtocolsFileOps { offset: usize, len: usize, buf: &mut [u8], - _data: MutexGuard, + mut data: MutexGuard, ) -> Result { - let content = Self::generate_protocols_content(); - proc_read(offset, len, buf, &content) + // `seq_file` (Linux `proc_create_net(&proto_seq_ops)`, `net/core/sock.c`): + // one fd sees one rendered record. + proc_read_snapshot(offset, len, buf, &mut data, || { + Ok(Self::generate_protocols_content()) + }) } } diff --git a/kernel/src/filesystem/procfs/pid/cgroup.rs b/kernel/src/filesystem/procfs/pid/cgroup.rs index 42f499ad59..00428f9bda 100644 --- a/kernel/src/filesystem/procfs/pid/cgroup.rs +++ b/kernel/src/filesystem/procfs/pid/cgroup.rs @@ -7,7 +7,7 @@ use crate::{ procfs::{ pid::ProcPidTarget, template::{Builder, FileOps, ProcFileBuilder}, - utils::proc_read, + utils::proc_read_snapshot, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }, @@ -54,9 +54,9 @@ impl FileOps for CgroupFileOps { offset: usize, len: usize, buf: &mut [u8], - _data: MutexGuard, + mut data: MutexGuard, ) -> Result { - let content = self.generate_content()?; - proc_read(offset, len, buf, &content) + // `single_open()`: one fd sees one cgroup record. + proc_read_snapshot(offset, len, buf, &mut data, || self.generate_content()) } } diff --git a/kernel/src/filesystem/procfs/pid/id_map.rs b/kernel/src/filesystem/procfs/pid/id_map.rs index 2aeced180c..08bb0a1917 100644 --- a/kernel/src/filesystem/procfs/pid/id_map.rs +++ b/kernel/src/filesystem/procfs/pid/id_map.rs @@ -8,6 +8,7 @@ use crate::{ procfs::{ pid::ProcPidTarget, template::{Builder, FileOps, ProcFileBuilder}, + utils::proc_read_snapshot, ProcfsFilePrivateData, }, vfs::{FilePrivateData, IndexNode, InodeMode}, @@ -368,34 +369,33 @@ impl FileOps for IdMapFileOps { offset: usize, len: usize, buf: &mut [u8], - data: MutexGuard, + mut data: MutexGuard, ) -> Result { - let user_ns = self.get_user_ns()?; + // Read the opener credential before `data` is mutably borrowed by the + // snapshot helper. The target namespace is resolved by the renderer, so + // a continuation read never touches the target task. let opener_cred = Self::open_cred_from_data(&data)?; - let inner = user_ns.inner.lock(); - let ctx = IdMapWriteContext { - map_type: self.map_type, - target_ns: user_ns.clone(), - opener_cred, - target_owner: inner.owner, - target_flags: inner.flags, - target_parent_could_setfcap: inner.parent_could_setfcap, - }; - let content = match self.map_type { - MapType::Uid => self.generate_content(&inner.uid_map, &ctx), - MapType::Gid => self.generate_content(&inner.gid_map, &ctx), - }; - - let content_bytes = content.as_bytes(); - if offset >= content_bytes.len() { - return Ok(0); - } + // Linux serves uid_map/gid_map through `seq_read()` (`fs/proc/base.c`), so + // the map text is frozen for the lifetime of the fd. + proc_read_snapshot(offset, len, buf, &mut data, || { + let user_ns = self.get_user_ns()?; + let inner = user_ns.inner.lock(); + let ctx = IdMapWriteContext { + map_type: self.map_type, + target_ns: user_ns.clone(), + opener_cred, + target_owner: inner.owner, + target_flags: inner.flags, + target_parent_could_setfcap: inner.parent_could_setfcap, + }; - let end = (offset + len).min(content_bytes.len()); - let to_copy = end - offset; - buf[..to_copy].copy_from_slice(&content_bytes[offset..end]); - Ok(to_copy) + Ok(match self.map_type { + MapType::Uid => self.generate_content(&inner.uid_map, &ctx), + MapType::Gid => self.generate_content(&inner.gid_map, &ctx), + } + .into_bytes()) + }) } fn write_at( @@ -474,29 +474,26 @@ impl FileOps for SetgroupsFileOps { offset: usize, len: usize, buf: &mut [u8], - _data: MutexGuard, + mut data: MutexGuard, ) -> Result { - let pcb = self - .target - .thread_group_leader() - .ok_or(SystemError::ESRCH)?; - let user_ns = pcb.cred().user_ns.clone(); - let inner = user_ns.inner.lock(); - - let content = if (inner.flags & USERNS_SETGROUPS_ALLOWED) != 0 { - "allow\n" - } else { - "deny\n" - }; - - let content_bytes = content.as_bytes(); - if offset >= content_bytes.len() { - return Ok(0); - } - let end = (offset + len).min(content_bytes.len()); - let to_copy = end - offset; - buf[..to_copy].copy_from_slice(&content_bytes[offset..end]); - Ok(to_copy) + // Linux `proc_setgroups_operations` reads through `seq_read()` + // (`fs/proc/base.c:3219`), so one fd sees one `allow`/`deny` record. + proc_read_snapshot(offset, len, buf, &mut data, || { + let allowed = { + let pcb = self + .target + .thread_group_leader() + .ok_or(SystemError::ESRCH)?; + let user_ns = pcb.cred().user_ns.clone(); + let inner = user_ns.inner.lock(); + (inner.flags & USERNS_SETGROUPS_ALLOWED) != 0 + }; + Ok(if allowed { + b"allow\n".to_vec() + } else { + b"deny\n".to_vec() + }) + }) } fn write_at( diff --git a/kernel/src/filesystem/procfs/pid/limits.rs b/kernel/src/filesystem/procfs/pid/limits.rs index 72ee3eed55..276a267105 100644 --- a/kernel/src/filesystem/procfs/pid/limits.rs +++ b/kernel/src/filesystem/procfs/pid/limits.rs @@ -10,7 +10,7 @@ use crate::{ procfs::{ pid::ProcPidTarget, template::{Builder, FileOps, ProcFileBuilder}, - utils::proc_read, + utils::proc_read_snapshot, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }, @@ -159,9 +159,11 @@ impl FileOps for LimitsFile { offset: usize, len: usize, buf: &mut [u8], - _data: MutexGuard, + mut data: MutexGuard, ) -> Result { - let content = self.generate_limits_content()?; - proc_read(offset, len, buf, content.as_bytes()) + // `single_open()`: one fd sees one rlimit record. + proc_read_snapshot(offset, len, buf, &mut data, || { + Ok(self.generate_limits_content()?.into_bytes()) + }) } } diff --git a/kernel/src/filesystem/procfs/pid/maps.rs b/kernel/src/filesystem/procfs/pid/maps.rs index de03061536..96d99578f8 100644 --- a/kernel/src/filesystem/procfs/pid/maps.rs +++ b/kernel/src/filesystem/procfs/pid/maps.rs @@ -9,7 +9,7 @@ use crate::{ procfs::{ pid::ProcPidTarget, template::{Builder, FileOps, ProcFileBuilder}, - utils::proc_read, + utils::proc_read_snapshot, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }, @@ -186,9 +186,12 @@ impl FileOps for MapsFileOps { offset: usize, len: usize, buf: &mut [u8], - _data: MutexGuard, + mut data: MutexGuard, ) -> Result { - let content = generate_maps_content(&self.target)?; - proc_read(offset, len, buf, &content) + // One fd sees one snapshot of the whole mapping table, so a mapping that + // appears while the reader is mid-stream cannot tear the byte stream. + proc_read_snapshot(offset, len, buf, &mut data, || { + generate_maps_content(&self.target) + }) } } diff --git a/kernel/src/filesystem/procfs/pid/mod.rs b/kernel/src/filesystem/procfs/pid/mod.rs index ed56b5a18a..788f7dc741 100644 --- a/kernel/src/filesystem/procfs/pid/mod.rs +++ b/kernel/src/filesystem/procfs/pid/mod.rs @@ -73,12 +73,29 @@ impl ProcPidTarget { Self { view_pid_ns, pid } } + /// Resolve `nr` as a thread-group id, the way Linux `next_tgid()` does. + /// + /// Requires the `PIDTYPE_TGID` link, which only the group leader holds, so + /// this resolves leaders only. Used for `/proc` directory *listing*, where + /// Linux also lists leaders only (`next_tgid()` in `fs/proc/base.c`). pub fn from_tgid_in_ns(view_pid_ns: Arc, pid: RawPid) -> Option { let target_pid = view_pid_ns.find_pid_in_ns(pid)?; target_pid.pid_task(PidType::TGID)?; Some(Self::new(view_pid_ns, target_pid)) } + /// Resolve `nr` as a task id, the way Linux `proc_pid_lookup()` does. + /// + /// `find_task_by_pid_ns()` goes through `PIDTYPE_PID`, which every task + /// (including a non-leader thread and a not-yet-reaped zombie) holds + /// through its own `thread_pid`, so any task with a live PID link can be + /// named by its tid. Used for `/proc/` *lookup*. + pub fn from_pid_in_ns(view_pid_ns: Arc, pid: RawPid) -> Option { + let target_pid = view_pid_ns.find_pid_in_ns(pid)?; + target_pid.pid_task(PidType::PID)?; + Some(Self::new(view_pid_ns, target_pid)) + } + pub fn from_task( view_pid_ns: Arc, task: Arc, @@ -158,7 +175,11 @@ impl PidDirOps { } pub(super) fn is_current_target(&self) -> bool { - ProcPidTarget::from_tgid_in_ns(self.target.view_pid_ns().clone(), self.target.vpid()) + // Cache-validity check: the same tid number must still resolve to the + // same `Arc`. It has to use the same rule as `lookup_child()`, + // otherwise a directory resolved by a non-leader tid would never match + // its cached entry and would be rebuilt on every lookup. + ProcPidTarget::from_pid_in_ns(self.target.view_pid_ns().clone(), self.target.vpid()) .map(|target| self.target.same_pid_object(&target)) .unwrap_or(false) } diff --git a/kernel/src/filesystem/procfs/pid/oom_score_adj.rs b/kernel/src/filesystem/procfs/pid/oom_score_adj.rs index 665064606e..20959d1664 100644 --- a/kernel/src/filesystem/procfs/pid/oom_score_adj.rs +++ b/kernel/src/filesystem/procfs/pid/oom_score_adj.rs @@ -6,7 +6,7 @@ use crate::{ procfs::{ pid::ProcPidTarget, template::{Builder, FileOps, ProcFileBuilder}, - utils::proc_read, + utils::{proc_read, proc_read_snapshot}, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }, @@ -53,12 +53,14 @@ impl FileOps for OomScoreFileOps { offset: usize, len: usize, buf: &mut [u8], - _data: MutexGuard, + mut data: MutexGuard, ) -> Result { - let pcb = self.target_process()?; - let score = crate::mm::oom::proc_oom_score(&pcb); - let content = format!("{}\n", score); - proc_read(offset, len, buf, content.as_bytes()) + // `single_open()`: one fd sees one oom_score record. + proc_read_snapshot(offset, len, buf, &mut data, || { + let pcb = self.target_process()?; + let score = crate::mm::oom::proc_oom_score(&pcb); + Ok(format!("{}\n", score).into_bytes()) + }) } } @@ -169,6 +171,10 @@ impl FileOps for OomScoreAdjFileOps { buf: &mut [u8], _data: MutexGuard, ) -> Result { + // Reverse guardrail: Linux serves `oom_score_adj` through `snprintf()` + + // `simple_read_from_buffer()` (`fs/proc/base.c`), not `seq_file`, so this + // read stays stream-style and must not be frozen per fd. Only + // `oom_score` (above) is a `single_open()` file. let pcb = self.target_process()?; let score = pcb.sig_info_irqsave().oom_score_adj(); let content = format!("{}\n", score); diff --git a/kernel/src/filesystem/procfs/pid/stat.rs b/kernel/src/filesystem/procfs/pid/stat.rs index 3000887143..c383d60eb6 100644 --- a/kernel/src/filesystem/procfs/pid/stat.rs +++ b/kernel/src/filesystem/procfs/pid/stat.rs @@ -11,7 +11,7 @@ use crate::{ procfs::{ pid::ProcPidTarget, template::{Builder, FileOps, ProcFileBuilder}, - utils::proc_read, + utils::proc_read_snapshot, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }, @@ -21,6 +21,7 @@ use crate::{ use alloc::{ string::{String, ToString}, sync::{Arc, Weak}, + vec::Vec, }; use system_error::SystemError; @@ -235,14 +236,12 @@ fn generate_linux_proc_stat_line(snapshot: &ProcStatSnapshot) -> String { line.finish() } -impl FileOps for StatFileOps { - fn read_at( - &self, - offset: usize, - len: usize, - buf: &mut [u8], - _data: MutexGuard, - ) -> Result { +impl StatFileOps { + /// Render the single line reported by this file. + /// + /// Linux serves `/proc//stat` through `single_open()`, so the line is + /// produced when the file is read and then frozen for that fd. + fn generate_content(&self) -> Result, SystemError> { let pcb = self.target.task().ok_or(SystemError::ESRCH)?; let (comm, user_vm) = { @@ -326,6 +325,18 @@ impl FileOps for StatFileOps { majflt: fault_usage.ru_majflt, cmajflt: child_usage.ru_majflt, }); - proc_read(offset, len, buf, content.as_bytes()) + Ok(content.into_bytes()) + } +} + +impl FileOps for StatFileOps { + fn read_at( + &self, + offset: usize, + len: usize, + buf: &mut [u8], + mut data: MutexGuard, + ) -> Result { + proc_read_snapshot(offset, len, buf, &mut data, || self.generate_content()) } } diff --git a/kernel/src/filesystem/procfs/pid/statm.rs b/kernel/src/filesystem/procfs/pid/statm.rs index a4741f3538..b2097d5692 100644 --- a/kernel/src/filesystem/procfs/pid/statm.rs +++ b/kernel/src/filesystem/procfs/pid/statm.rs @@ -9,7 +9,7 @@ use crate::{ procfs::{ pid::ProcPidTarget, template::{Builder, FileOps, ProcFileBuilder}, - utils::proc_read, + utils::proc_read_snapshot, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }, @@ -42,35 +42,38 @@ impl FileOps for StatmFileOps { offset: usize, len: usize, buf: &mut [u8], - _data: MutexGuard, + mut data: MutexGuard, ) -> Result { - let pcb = self - .target - .thread_group_leader() - .ok_or(SystemError::ESRCH)?; + // `single_open()`: one fd sees one statm record. + proc_read_snapshot(offset, len, buf, &mut data, || { + let pcb = self + .target + .thread_group_leader() + .ok_or(SystemError::ESRCH)?; - let user_vm = { - let basic = pcb.basic(); - basic.user_vm() - }; + let user_vm = { + let basic = pcb.basic(); + basic.user_vm() + }; - // 获取进程内存信息（简化实现） - let (size_pages, resident_pages) = user_vm - .map(|vm| { - let guard = vm.read_guard_no_reservations(); - // statm 第一列为总虚拟内存页数，第二列使用 OOM/RSS 维护的常驻页计数。 - let size_pages = (guard - .vma_usage_bytes() - .saturating_add(MMArch::PAGE_SIZE - 1)) - >> MMArch::PAGE_SHIFT; - (size_pages, vm.resident_pages()) - }) - .unwrap_or((0, 0)); + // Process memory information (simplified: no shared/text/lib/data). + let (size_pages, resident_pages) = user_vm + .map(|vm| { + let guard = vm.read_guard_no_reservations(); + // Field 1 is total virtual pages; field 2 uses the RAS-maintained resident page count. + let size_pages = (guard + .vma_usage_bytes() + .saturating_add(MMArch::PAGE_SIZE - 1)) + >> MMArch::PAGE_SHIFT; + (size_pages, vm.resident_pages()) + }) + .unwrap_or((0, 0)); - // statm 格式: size resident shared text lib data dt - // 简化实现，只返回 size/resident，其他字段为 0 - let content = format!("{} {} 0 0 0 0 0\n", size_pages, resident_pages); + // statm layout: size resident shared text lib data dt. + // Only size/resident are implemented; the remaining fields stay 0. + let content = format!("{} {} 0 0 0 0 0\n", size_pages, resident_pages); - proc_read(offset, len, buf, content.as_bytes()) + Ok(content.into_bytes()) + }) } } diff --git a/kernel/src/filesystem/procfs/pid/status.rs b/kernel/src/filesystem/procfs/pid/status.rs index 2ed1af9d87..70aba5b8ff 100644 --- a/kernel/src/filesystem/procfs/pid/status.rs +++ b/kernel/src/filesystem/procfs/pid/status.rs @@ -8,7 +8,7 @@ use crate::{ procfs::{ pid::ProcPidTarget, template::{Builder, FileOps, ProcFileBuilder}, - utils::{proc_read, trim_string}, + utils::{proc_read_snapshot, trim_string}, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }, @@ -219,11 +219,13 @@ impl FileOps for StatusFileOps { offset: usize, len: usize, buf: &mut [u8], - _data: MutexGuard, + mut data: MutexGuard, ) -> Result { - let content = self.generate_status_content()?; - // log::info!("Generated /proc/[pid]/status content"); - - proc_read(offset, len, buf, &content) + // Linux serves `/proc//status` through `single_open()`, so one fd + // sees one record: EOF stays EOF even while `Time`/`Stime`/`vrtime` + // keep growing, and repositioning re-renders. + proc_read_snapshot(offset, len, buf, &mut data, || { + self.generate_status_content() + }) } } diff --git a/kernel/src/filesystem/procfs/root.rs b/kernel/src/filesystem/procfs/root.rs index 7e89bdf228..4af0b9fe1e 100644 --- a/kernel/src/filesystem/procfs/root.rs +++ b/kernel/src/filesystem/procfs/root.rs @@ -110,10 +110,16 @@ impl DirOps for RootDirOps { dir: &ProcDir, name: &str, ) -> Result, SystemError> { + // Lookup goes through the PID link, like Linux `proc_pid_lookup()` + // (`find_task_by_pid_ns()`), so any task that still holds a PID link can + // be named by its own tid. Directory listing keeps using the TGID link + // in `populate_children()` and therefore yields leaders only, matching + // Linux `next_tgid()`. The asymmetry between lookup and listing is + // intentional and matches Linux. // 首先检查是否是 PID 目录 if let Ok(pid) = name.parse::() { // 检查进程是否存在 - if let Some(target) = crate::filesystem::procfs::pid::ProcPidTarget::from_tgid_in_ns( + if let Some(target) = crate::filesystem::procfs::pid::ProcPidTarget::from_pid_in_ns( self.pid_ns.clone(), pid, ) { @@ -163,8 +169,21 @@ impl DirOps for RootDirOps { // 获取缓存写锁并填充 let mut cached_children = dir.cached_children().write(); + // A numeric entry stays valid only while its tid still resolves as a + // thread group id. `lookup_child()` creates entries through the PID link + // (any live task), so without this filter a directory opened by looking + // up a non-leader tid would leak into the listing. Linux `next_tgid()` + // lists leaders only, and so must this. cached_children.retain(|name, child| { - name.parse::().is_err() || self.validate_child(child.as_ref()) + let Ok(pid) = name.parse::() else { + return true; + }; + self.validate_child(child.as_ref()) + && crate::filesystem::procfs::pid::ProcPidTarget::from_tgid_in_ns( + self.pid_ns.clone(), + pid, + ) + .is_some() }); // 填充进程目录（只传递 PID） diff --git a/kernel/src/filesystem/procfs/stat.rs b/kernel/src/filesystem/procfs/stat.rs index 5d04a38afd..d1b3bcd9a7 100644 --- a/kernel/src/filesystem/procfs/stat.rs +++ b/kernel/src/filesystem/procfs/stat.rs @@ -4,7 +4,7 @@ use crate::{ filesystem::{ procfs::{ template::{Builder, FileOps, ProcFileBuilder}, - utils::{proc_read, trim_string}, + utils::{proc_read_snapshot, trim_string}, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }, @@ -135,9 +135,12 @@ impl FileOps for StatFileOps { offset: usize, len: usize, buf: &mut [u8], - _data: MutexGuard, + mut data: MutexGuard, ) -> Result { - let content = Self::generate_stat_content(); - proc_read(offset, len, buf, &content) + // `seq_file`: Linux opens `/proc/stat` through `single_open_size()` + // (`fs/proc/stat.c`), so one fd sees one rendered record. + proc_read_snapshot(offset, len, buf, &mut data, || { + Ok(Self::generate_stat_content()) + }) } } diff --git a/kernel/src/filesystem/procfs/utils.rs b/kernel/src/filesystem/procfs/utils.rs index d3e908866c..8953eb437a 100644 --- a/kernel/src/filesystem/procfs/utils.rs +++ b/kernel/src/filesystem/procfs/utils.rs @@ -1,6 +1,8 @@ use alloc::vec::Vec; use system_error::SystemError; +use crate::{filesystem::vfs::FilePrivateData, libs::mutex::MutexGuard}; + /// 去除Vec中所有的\0,并在结尾添加\0 #[inline] pub(super) fn trim_string(data: &mut Vec) { @@ -28,3 +30,57 @@ pub(super) fn proc_read( buf[0..src.len()].copy_from_slice(src); return Ok(src.len()); } + +/// Snapshot read for procfs: mirrors Linux `seq_file` (`single_open()` / +/// `seq_read_iter()` / `seq_lseek()`). +/// +/// One fd re-renders only on the first read, or when the read position no longer +/// matches the continuation position: +/// +/// - `offset == read_pos` and `offset != 0`: continue from the snapshot, so +/// **content growing between two reads does not revive EOF**; +/// - `offset == 0`: re-render, matching `seq_read_iter()`'s `ki_pos == 0` reset; +/// - anything else: re-render and reposition at `offset`, matching `traverse()`. +/// +/// `render` is only called when needed (the continuation path never re-renders). +/// A failed render drops the continuation point, the way a failed `traverse()` +/// resets the buffer (`fs/seq_file.c:196-203`), so the next read re-renders +/// instead of serving a stale snapshot. +pub(super) fn proc_read_snapshot( + offset: usize, + len: usize, + buf: &mut [u8], + data: &mut MutexGuard, + render: F, +) -> Result +where + F: FnOnce() -> Result, SystemError>, +{ + let FilePrivateData::Procfs(pdata) = &mut **data else { + // A few callers (e.g. symlink reads) reach read_at() without procfs + // private data. Fall back to render-per-read, i.e. the previous behaviour. + let content = render()?; + return proc_read(offset, len, buf, &content); + }; + + let pos = match pdata.read_pos { + Some(pos) if pos == offset && offset != 0 => pos, + _ => { + match render() { + Ok(rendered) => { + pdata.data = rendered; + pdata.read_pos = Some(offset); + } + Err(err) => { + pdata.read_pos = None; + return Err(err); + } + } + offset + } + }; + + let n = proc_read(pos, len, buf, &pdata.data)?; + pdata.read_pos = Some(pos + n); + Ok(n) +} diff --git a/kernel/src/filesystem/procfs/version.rs b/kernel/src/filesystem/procfs/version.rs index 8abedb6949..f2fd12ce82 100644 --- a/kernel/src/filesystem/procfs/version.rs +++ b/kernel/src/filesystem/procfs/version.rs @@ -7,7 +7,7 @@ use crate::{ filesystem::{ procfs::{ template::{Builder, FileOps, ProcFileBuilder}, - utils::proc_read, + utils::proc_read_snapshot, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }, @@ -57,9 +57,11 @@ impl FileOps for VersionFileOps { offset: usize, len: usize, buf: &mut [u8], - _data: MutexGuard, + mut data: MutexGuard, ) -> Result { - let content = Self::generate_version_content(); - proc_read(offset, len, buf, &content) + // `single_open()`: one fd sees one version record. + proc_read_snapshot(offset, len, buf, &mut data, || { + Ok(Self::generate_version_content()) + }) } } diff --git a/kernel/src/filesystem/procfs/version_signature.rs b/kernel/src/filesystem/procfs/version_signature.rs index eda65462a5..10d8cf5d5f 100644 --- a/kernel/src/filesystem/procfs/version_signature.rs +++ b/kernel/src/filesystem/procfs/version_signature.rs @@ -6,7 +6,7 @@ use crate::filesystem::{ procfs::{ template::{Builder, FileOps, ProcFileBuilder}, - utils::proc_read, + utils::proc_read_snapshot, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }; @@ -35,8 +35,12 @@ impl FileOps for VersionSignatureFileOps { offset: usize, len: usize, buf: &mut [u8], - _data: MutexGuard, + mut data: MutexGuard, ) -> Result { - proc_read(offset, len, buf, Self::VERSION_SIGNATURE) + // Like the other one-record procfs files, this keeps a single record per + // fd, so a later read cannot observe a different version string. + proc_read_snapshot(offset, len, buf, &mut data, || { + Ok(Self::VERSION_SIGNATURE.to_vec()) + }) } } diff --git a/kernel/src/filesystem/procfs/vmstat.rs b/kernel/src/filesystem/procfs/vmstat.rs index 965d520ac7..bf8d8c2590 100644 --- a/kernel/src/filesystem/procfs/vmstat.rs +++ b/kernel/src/filesystem/procfs/vmstat.rs @@ -5,7 +5,7 @@ use crate::{ filesystem::{ procfs::{ template::{Builder, FileOps, ProcFileBuilder}, - utils::{proc_read, trim_string}, + utils::{proc_read_snapshot, trim_string}, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }, @@ -492,9 +492,12 @@ impl FileOps for VmstatFileOps { offset: usize, len: usize, buf: &mut [u8], - _data: MutexGuard, + mut data: MutexGuard, ) -> Result { - let content = Self::generate_vmstat_content(); - proc_read(offset, len, buf, &content) + // `seq_file` (Linux `proc_create_seq(&vmstat_op)`, `mm/vmstat.c`): one fd + // sees one rendered record. + proc_read_snapshot(offset, len, buf, &mut data, || { + Ok(Self::generate_vmstat_content()) + }) } } diff --git a/kernel/src/process/task.rs b/kernel/src/process/task.rs index 64046ea1ca..d85c4648bf 100644 --- a/kernel/src/process/task.rs +++ b/kernel/src/process/task.rs @@ -225,8 +225,13 @@ pub struct ProcessControlBlock { pub(super) seccomp_filter: SpinLock>>, /// Parent process pointer. + /// + /// Per-task: every task gets its own copy at fork (see the `CLONE_PARENT` / + /// `CLONE_THREAD` branches in `fork.rs`), and re-parenting updates the + /// whole thread group member by member, the way Linux + /// `forget_original_parent()` walks `for_each_thread()`. pub(super) parent_pcb: RwLock>, - /// Real (original) parent process pointer. + /// Real (original) parent process pointer. Per-task, see `parent_pcb`. pub(super) real_parent_pcb: RwLock>, /// The natural parent pointer for wait operations. /// @@ -234,6 +239,11 @@ pub struct ProcessControlBlock { /// task_struct::parent, whereas DragonOS models parent_pcb/real_parent_pcb /// on the thread-group leader. This field preserves the thread-level parent /// relationship required by wait. + /// + /// Re-parenting updates it together with the other three pointers. The only + /// Linux difference is the `if (likely(!t->ptrace))` guard: a traced task's + /// `->parent` is its tracer. DragonOS keeps tracing in the ptrace relation + /// table instead, so there is nothing to guard here today. pub(super) wait_parent_pcb: RwLock>, /// Thread-level natural-parent compensation for PTRACE_TRACEME. /// @@ -1293,19 +1303,54 @@ impl ProcessControlBlock { } } + /// Update the parent links of one task. + /// + /// The four parent pointers are per-task state (`fork.rs` copies them into + /// every newly created task, and `CLONE_THREAD` children inherit them + /// independently), so a thread group is only fully re-parented when every + /// member is updated. Linux does the same with `for_each_thread()` in + /// `forget_original_parent()`. + /// + /// Linux additionally guards the wait parent with `if (likely(!t->ptrace))`: + /// a traced task's `->parent` is its tracer, not the new parent. DragonOS + /// keeps the tracer relationship in `process::ptrace`'s relation table + /// (`ptracer_of()`) and never writes it into `wait_parent_pcb`, so the + /// unconditional update is equivalent today. If tracing is ever modelled + /// through these fields, the guard has to be added here as well. + fn reparent_one_task_locked( + task: &Arc, + new_parent: &Arc, + parent_pid_in_child_ns: RawPid, + ) { + *task.parent_pcb.write_irqsave() = Arc::downgrade(new_parent); + *task.real_parent_pcb.write_irqsave() = Arc::downgrade(new_parent); + *task.wait_parent_pcb.write_irqsave() = Arc::downgrade(new_parent); + *task.fork_parent_pcb.write_irqsave() = Arc::downgrade(new_parent); + + // `basic.ppid` has no reader today (`stat`/`status` derive Ppid from + // `parent_pcb()`), but it belongs to the same parent record and must + // stay consistent with the pointers above. + task.basic.write_irqsave().ppid = parent_pid_in_child_ns; + } + + /// Attach a whole child thread group to `new_parent`. + /// + /// Only the group leader is registered in `children`, so the caller passes + /// the leader; the leader's group list is then walked to update every + /// member. Updating just the leader would make + /// `/proc//task//status` report a different `Ppid` than + /// `/proc//status` for the same thread group. fn reparent_child_to_locked( child: &Arc, new_parent: &Arc, ) { - *child.parent_pcb.write_irqsave() = Arc::downgrade(new_parent); - *child.real_parent_pcb.write_irqsave() = Arc::downgrade(new_parent); - *child.wait_parent_pcb.write_irqsave() = Arc::downgrade(new_parent); - *child.fork_parent_pcb.write_irqsave() = Arc::downgrade(new_parent); - let parent_pid_in_child_ns = new_parent .task_pid_nr_ns(PidType::PID, Some(child.active_pid_ns())) .unwrap_or(RawPid::new(0)); - child.basic.write_irqsave().ppid = parent_pid_in_child_ns; + + for task in ProcessManager::thread_group_tasks_snapshot(child.clone()) { + Self::reparent_one_task_locked(&task, new_parent, parent_pid_in_child_ns); + } ProcessControlBlock::link_child_to_parent_list(child, new_parent); diff --git a/user/apps/tests/dunitest/suites/normal/proc_task_status.cc b/user/apps/tests/dunitest/suites/normal/proc_task_status.cc index 31d54d2370..d1566bb988 100644 --- a/user/apps/tests/dunitest/suites/normal/proc_task_status.cc +++ b/user/apps/tests/dunitest/suites/normal/proc_task_status.cc @@ -64,12 +64,12 @@ class UniqueFd { int fd_ = -1; }; -// Procfs files are regenerated on every read_at() while the file position stays -// a plain byte offset, so a second read() at a stale offset can hand back tail -// bytes of a longer re-render. One read(2), with a buffer far larger than these -// files, therefore yields exactly one coherent snapshot; callers that need the -// whole file (and every compared field is checked for presence) would notice if -// the content ever outgrew the buffer. +// Seq-family procfs files now hand out one frozen snapshot per open(): the +// first read() renders the record and later reads replay the remainder of that +// same buffer, so a single read(2) with a buffer far larger than these files +// yields exactly one coherent snapshot. Every compared field is checked for +// presence, so a truncated snapshot would be reported as a missing field rather +// than silently passing. constexpr size_t kSnapshotBufSize = 4096; bool ReadProcSnapshot(const std::string& path, std::string* out, int* err_out) { @@ -422,8 +422,9 @@ TEST(ProcTaskStatus, NonLeaderThreadStatusDescribesThread) { EXPECT_EQ(std::to_string(pid), Field(fields, "Tgid")); EXPECT_NE("0", Field(fields, "Tgid")); // A thread inherits its creator's parent, so this must agree with the - // process view while the parent is alive. Reparenting only rewrites the - // group leader today, which is a separate pre-existing gap. + // process view while the parent is alive. Re-parenting has to rewrite every + // thread as well; that direction is covered by + // ProcfsTaskSemantics.ThreadPpidFollowsGroupReparent. EXPECT_EQ(Field(ParseStatus(proc_before), "Ppid"), Field(fields, "Ppid")); // The thread state must come from the worker (blocked), not from the leader. diff --git a/user/apps/tests/dunitest/suites/normal/procfs_task_semantics.cc b/user/apps/tests/dunitest/suites/normal/procfs_task_semantics.cc new file mode 100644 index 0000000000..98d0b50586 --- /dev/null +++ b/user/apps/tests/dunitest/suites/normal/procfs_task_semantics.cc @@ -0,0 +1,926 @@ +// procfs task semantics (issue #2283). +// +// Three behaviours are pinned here, each against the Linux 6.6 model: +// +// 1. one fd sees one record. Linux serves these files through +// single_open()/seq_read_iter(), so a read() that reached EOF keeps +// returning 0 even while the record grows, and moving the file position +// re-renders. Before the fix every read() re-rendered and the stale byte +// offset sliced the *new* render, so a second read() could hand back tail +// bytes of a longer record; +// 2. re-parenting a thread group rewrites the parent links of every thread, +// so /proc//task//status and /proc//status agree on Ppid; +// 3. /proc/ resolves any task that still holds a PID link (Linux +// proc_pid_lookup() -> find_task_by_pid_ns()), while /proc *lists* group +// leaders only (Linux next_tgid()). +// +// Companion analysis: +// docs/kernel/filesystem/proc/procfs-task-semantics-root-cause.md +// docs/kernel/filesystem/proc/procfs-task-semantics-fix-plan.md + +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +namespace { + +constexpr size_t kReadChunk = 128; +constexpr int kPollTimeoutMs = 2000; + +class UniqueFd { +public: + UniqueFd() = default; + explicit UniqueFd(int fd) : fd_(fd) {} + ~UniqueFd() { Reset(); } + UniqueFd(const UniqueFd&) = delete; + UniqueFd& operator=(const UniqueFd&) = delete; + + int get() const { return fd_; } + bool valid() const { return fd_ >= 0; } + + void Reset(int fd = -1) { + if (fd_ >= 0) { + close(fd_); + } + fd_ = fd; + } + +private: + int fd_ = -1; +}; + +std::string Escape(const std::string& s) { + std::string out; + out.reserve(s.size() + 16); + for (char c : s) { + if (c == '\n') { + out += "\\n"; + } else if (c == '\t') { + out += "\\t"; + } else if (static_cast(c) < 0x20) { + char hex[8]; + snprintf(hex, sizeof(hex), "\\x%02x", static_cast(c)); + out += hex; + } else { + out.push_back(c); + } + } + return out; +} + +std::string Trim(std::string s) { + auto not_space = [](unsigned char c) { + return c != ' ' && c != '\t' && c != '\n' && c != '\r' && c != '\0'; + }; + s.erase(s.begin(), std::find_if(s.begin(), s.end(), not_space)); + s.erase(std::find_if(s.rbegin(), s.rend(), not_space).base(), s.end()); + return s; +} + +// Read `fd` until EOF in `chunk`-sized pieces, appending the bytes to `out`. +// Returns the errno of a failed read, or 0 on success. +int ReadToEof(int fd, size_t chunk, std::string* out) { + char buf[256]; + if (chunk > sizeof(buf)) { + chunk = sizeof(buf); + } + for (;;) { + const ssize_t n = read(fd, buf, chunk); + if (n == 0) { + return 0; + } + if (n < 0) { + if (errno == EINTR) { + continue; + } + return errno; + } + out->append(buf, static_cast(n)); + } +} + +bool ReadWholePath(const std::string& path, std::string* out, int* err_out) { + UniqueFd fd(open(path.c_str(), O_RDONLY)); + if (!fd.valid()) { + *err_out = errno; + return false; + } + *err_out = ReadToEof(fd.get(), kReadChunk, out); + return *err_out == 0; +} + +// Shape of a record: the number of whitespace-separated tokens on each line. +// Two renders of the same file agree on this even when the numeric values (and +// therefore the byte length) moved, which is what makes it usable as the +// structural half of the chunked-read check. +std::vector Shape(const std::string& text) { + std::vector shape; + size_t pos = 0; + while (pos < text.size()) { + size_t nl = text.find('\n', pos); + if (nl == std::string::npos) { + nl = text.size(); + } + size_t tokens = 0; + bool in_token = false; + for (size_t i = pos; i < nl; ++i) { + const char c = text[i]; + const bool space = (c == ' ' || c == '\t' || c == '\0' || c == '\r'); + if (!space && !in_token) { + ++tokens; + } + in_token = !space; + } + shape.push_back(tokens); + pos = nl + 1; + } + return shape; +} + +std::map ParseStatus(const std::string& text) { + std::map fields; + size_t pos = 0; + while (pos <= text.size()) { + const size_t nl = text.find('\n', pos); + std::string line = (nl == std::string::npos) ? text.substr(pos) : text.substr(pos, nl - pos); + pos = (nl == std::string::npos) ? text.size() + 1 : nl + 1; + + std::string clean; + for (char c : line) { + if (c != '\0') { + clean.push_back(c); + } + } + const size_t colon = clean.find(':'); + if (colon == std::string::npos) { + continue; + } + fields[clean.substr(0, colon)] = Trim(clean.substr(colon + 1)); + } + return fields; +} + +// Linux spells the field "PPid", DragonOS renders it as "Ppid"; both names must +// resolve to the same value so the assertions below describe the semantics and +// not the spelling. +std::string Field(const std::map& fields, const char* key) { + auto it = fields.find(key); + if (it != fields.end()) { + return it->second; + } + if (strcmp(key, "Ppid") == 0) { + it = fields.find("PPid"); + } else if (strcmp(key, "PPid") == 0) { + it = fields.find("Ppid"); + } + return it == fields.end() ? std::string() : it->second; +} + +std::string StatusPath(pid_t pid, const char* suffix = "status") { + char path[64]; + snprintf(path, sizeof(path), "/proc/%d/%s", pid, suffix); + return std::string(path); +} + +std::string TaskStatusPath(pid_t pid, long tid, const char* suffix = "status") { + char path[80]; + snprintf(path, sizeof(path), "/proc/%d/task/%ld/%s", pid, tid, suffix); + return std::string(path); +} + +std::string TidPath(long tid, const char* suffix = "") { + char path[64]; + snprintf(path, sizeof(path), "/proc/%ld/%s", tid, suffix); + return std::string(path); +} + +long GetTid() { + return static_cast(syscall(SYS_gettid)); +} + +std::vector ListDir(const std::string& path) { + std::vector names; + DIR* d = opendir(path.c_str()); + if (d == nullptr) { + return names; + } + while (struct dirent* e = readdir(d)) { + if (strcmp(e->d_name, ".") == 0 || strcmp(e->d_name, "..") == 0) { + continue; + } + names.push_back(e->d_name); + } + closedir(d); + std::sort(names.begin(), names.end()); + return names; +} + +bool Contains(const std::vector& v, const std::string& needle) { + return std::find(v.begin(), v.end(), needle) != v.end(); +} + +// RAII wrapper around PR_SET_NAME. The comm shows up verbatim in the first line +// of /proc//status and in /proc//stat, so changing it between two +// reads is a deterministic way to make a record grow without depending on +// timing. +class CommGuard { +public: + explicit CommGuard(const char* name) { + char buf[16] = {0}; + if (prctl(PR_GET_NAME, buf, 0, 0, 0) == 0) { + saved_ = buf; + } + prctl(PR_SET_NAME, name, 0, 0, 0); + } + ~CommGuard() { + if (!saved_.empty()) { + prctl(PR_SET_NAME, saved_.c_str(), 0, 0, 0); + } + } + CommGuard(const CommGuard&) = delete; + CommGuard& operator=(const CommGuard&) = delete; + + static bool Set(const char* name) { return prctl(PR_SET_NAME, name, 0, 0, 0) == 0; } + +private: + std::string saved_; +}; + +// Long-lived worker thread: publishes its tid, then blocks until the owner +// closes the release pipe. The destructor always joins, so a failing ASSERT +// cannot leave a blocked thread behind for the next case. +class Worker { +public: + Worker() = default; + ~Worker() { Stop(); } + Worker(const Worker&) = delete; + Worker& operator=(const Worker&) = delete; + + bool Start() { + if (pipe(ready_) != 0 || pipe(release_) != 0) { + return false; + } + if (pthread_create(&thread_, nullptr, Main, this) != 0) { + return false; + } + started_ = true; + char byte = 0; + ssize_t n = 0; + do { + n = read(ready_[0], &byte, 1); + } while (n < 0 && errno == EINTR); + if (n != 1) { + Stop(); + return false; + } + return true; + } + + void Stop() { + if (release_[0] >= 0) { + close(release_[0]); + release_[0] = -1; + } + if (release_[1] >= 0) { + close(release_[1]); + release_[1] = -1; + } + if (started_) { + pthread_join(thread_, nullptr); + started_ = false; + } + if (ready_[0] >= 0) { + close(ready_[0]); + ready_[0] = -1; + } + if (ready_[1] >= 0) { + close(ready_[1]); + ready_[1] = -1; + } + } + + long tid() const { return tid_; } + +private: + static void* Main(void* arg) { + Worker* self = static_cast(arg); + prctl(PR_SET_NAME, "worker", 0, 0, 0); + self->tid_ = GetTid(); + const char ready = 'r'; + if (write(self->ready_[1], &ready, 1) != 1) { + return nullptr; + } + char buf[8]; + while (read(self->release_[0], buf, sizeof(buf)) > 0) { + } + return nullptr; + } + + int ready_[2] = {-1, -1}; + int release_[2] = {-1, -1}; + long tid_ = 0; + pthread_t thread_ = {}; + bool started_ = false; +}; + +bool WaitForExit(pid_t pid) { + for (int i = 0; i < kPollTimeoutMs / 10; ++i) { + int status = 0; + if (waitpid(pid, &status, WNOHANG) == pid) { + return true; + } + usleep(10000); + } + return false; +} + +// Kills and reaps a forked child on scope exit unless it was disarmed. Cases +// that fork a paused child before their last assertion use this instead of a +// trailing kill(), so a failing ASSERT cannot leave that child running (and +// blocking) for the rest of the suite. +class ReapedChild { +public: + explicit ReapedChild(pid_t pid) : pid_(pid) {} + ~ReapedChild() { Reap(); } + ReapedChild(const ReapedChild&) = delete; + ReapedChild& operator=(const ReapedChild&) = delete; + + void Disarm() { pid_ = -1; } + +private: + void Reap() { + if (pid_ > 0) { + kill(pid_, SIGKILL); + WaitForExit(pid_); + pid_ = -1; + } + } + + pid_t pid_ = -1; +}; + +} // namespace + +// --------------------------------------------------------------------------- +// 1. one fd == one record +// --------------------------------------------------------------------------- + +// Reading to EOF and then reading again must stay at EOF, even when the record +// grew in between. /proc//status is a single_open() file, and its Name +// field lets the test grow the record by an exact number of bytes instead of +// hoping that a counter crosses a digit boundary. +TEST(ProcfsTaskSemantics, EofStaysEofWhileContentGrows) { + CommGuard guard("aaa"); + + UniqueFd fd(open("/proc/self/status", O_RDONLY)); + ASSERT_TRUE(fd.valid()) << "cannot open /proc/self/status: errno=" << errno; + + std::string chunked; + ASSERT_EQ(0, ReadToEof(fd.get(), 13, &chunked)) << "chunked read failed"; + ASSERT_FALSE(chunked.empty()); + + ASSERT_TRUE(CommGuard::Set("aaa_procfs_x")) << "cannot grow the record"; + std::string fresh; + int err = 0; + ASSERT_TRUE(ReadWholePath("/proc/self/status", &fresh, &err)) + << "cannot re-read /proc/self/status: errno=" << err; + ASSERT_GT(fresh.size(), chunked.size()) + << "the record did not grow, so this case cannot observe the bug: " + << Escape(chunked) << " vs " << Escape(fresh); + + char tail[kReadChunk]; + errno = 0; + const ssize_t extra = read(fd.get(), tail, sizeof(tail)); + ASSERT_GE(extra, 0) << "read after EOF failed: errno=" << errno; + EXPECT_EQ(0, extra) << "the fd revived EOF and served " << extra + << " byte(s) of a longer render: \"" << Escape(std::string(tail, extra)) + << "\""; +} + +// lseek(0) must re-render: the reader sees the current record, not the frozen +// one, matching seq_read_iter()'s ki_pos == 0 reset. +TEST(ProcfsTaskSemantics, RewindRerendersFreshRecord) { + CommGuard guard("aaa"); + + UniqueFd fd(open("/proc/self/status", O_RDONLY)); + ASSERT_TRUE(fd.valid()) << "cannot open /proc/self/status: errno=" << errno; + std::string first; + ASSERT_EQ(0, ReadToEof(fd.get(), kReadChunk, &first)); + + ASSERT_TRUE(CommGuard::Set("aaa_procfs_x")); + std::string fresh; + int err = 0; + ASSERT_TRUE(ReadWholePath("/proc/self/status", &fresh, &err)) << "errno=" << err; + ASSERT_GT(fresh.size(), first.size()) << "the record did not grow"; + + EXPECT_EQ(std::string::npos, first.find("aaa_procfs_x")); + EXPECT_NE(std::string::npos, fresh.find("aaa_procfs_x")); + + ASSERT_EQ(0, lseek(fd.get(), 0, SEEK_SET)) << "lseek(0) failed: errno=" << errno; + std::string reread; + ASSERT_EQ(0, ReadToEof(fd.get(), kReadChunk, &reread)); + // The current record is identified by its Name field. Exact bytes and byte + // counts are both unusable here: Time/Stime/vrtime advance between the two + // reads, and the widths of the counters around them move in either + // direction, so only the field itself distinguishes a re-render from the + // frozen snapshot (whose Name is "aaa"). + EXPECT_EQ("aaa_procfs_x", Field(ParseStatus(reread), "Name")) + << "a rewound fd must serve the current record: " << Escape(reread); +} + +// A read position that is neither 0 nor the continuation position re-renders +// the record and serves it from that offset; at or past the end it serves +// nothing. /proc/version never changes, so the expected bytes are exact. +TEST(ProcfsTaskSemantics, SeekIntoRecordServesRecordBytes) { + const char* kPath = "/proc/version"; + std::string first; + int err = 0; + ASSERT_TRUE(ReadWholePath(kPath, &first, &err)) << kPath << ": errno=" << err; + ASSERT_FALSE(first.empty()); + + UniqueFd fd(open(kPath, O_RDONLY)); + ASSERT_TRUE(fd.valid()) << "cannot open " << kPath << ": errno=" << errno; + std::string full; + ASSERT_EQ(0, ReadToEof(fd.get(), kReadChunk, &full)); + ASSERT_EQ(first, full); + + const size_t length = first.size(); + const size_t offsets[] = {0, length / 2, length - 1, length, length + 7}; + for (size_t offset : offsets) { + ASSERT_EQ(static_cast(offset), lseek(fd.get(), static_cast(offset), SEEK_SET)) + << "lseek(" << offset << ") failed: errno=" << errno; + std::string got; + ASSERT_EQ(0, ReadToEof(fd.get(), kReadChunk, &got)) << "offset=" << offset; + const std::string expected = offset < length ? first.substr(offset) : std::string(); + EXPECT_EQ(expected, got) << "offset=" << offset << " length=" << length; + } +} + +// Guard for the VFS-level behaviour the fix relies on: a procfs lseek(SEEK_END) +// is EINVAL, like seq_lseek(). +TEST(ProcfsTaskSemantics, SeekEndIsEinval) { + UniqueFd fd(open("/proc/self/status", O_RDONLY)); + ASSERT_TRUE(fd.valid()) << "cannot open /proc/self/status: errno=" << errno; + errno = 0; + EXPECT_EQ(-1, lseek(fd.get(), 0, SEEK_END)); + EXPECT_EQ(EINVAL, errno); +} + +// Every file the fix converted to snapshot reads must survive a chunked read: +// the chunks must stop at EOF, and a second read on the same fd must stay +// there. Byte equality is only required for records that cannot move while the +// test runs; the rest are compared structurally, because their numbers change +// between two reads on Linux as well. +// +// uid_map/gid_map are deliberately absent: reading the *init* user namespace's +// own map deadlocks DragonOS before this change as well (read_at holds +// UserNamespace::inner and generate_content() re-locks the same namespace, +// because an init namespace has no parent to display). That is a separate +// pre-existing bug, so it must not be pinned here. +TEST(ProcfsTaskSemantics, SnapshotSurvivesChunkedRead) { + const char* const kStrict[] = { + "/proc/version", + "/proc/version_signature", + "/proc/cmdline", + "/proc/self/cgroup", + "/proc/self/limits", + }; + const char* const kStructural[] = { + "/proc/self/status", + "/proc/self/stat", + "/proc/self/statm", + "/proc/self/maps", + "/proc/self/mountinfo", + "/proc/stat", + "/proc/meminfo", + "/proc/vmstat", + "/proc/loadavg", + "/proc/cpuinfo", + "/proc/net/arp", + "/proc/net/protocols", + }; + + for (const char* path : kStrict) { + UniqueFd fd(open(path, O_RDONLY)); + ASSERT_TRUE(fd.valid()) << "cannot open " << path << ": errno=" << errno; + std::string chunked; + ASSERT_EQ(0, ReadToEof(fd.get(), 4, &chunked)) << path; + char tail[16]; + EXPECT_EQ(0, read(fd.get(), tail, sizeof(tail))) << path << ": EOF was revived"; + + std::string fresh; + int err = 0; + ASSERT_TRUE(ReadWholePath(path, &fresh, &err)) << path << ": errno=" << err; + EXPECT_EQ(fresh, chunked) << path << ": a frozen fd must serve its own record"; + } + + for (const char* path : kStructural) { + UniqueFd fd(open(path, O_RDONLY)); + ASSERT_TRUE(fd.valid()) << "cannot open " << path << ": errno=" << errno; + std::string chunked; + ASSERT_EQ(0, ReadToEof(fd.get(), 4, &chunked)) << path; + char tail[16]; + EXPECT_EQ(0, read(fd.get(), tail, sizeof(tail))) << path << ": EOF was revived"; + + std::string fresh; + int err = 0; + ASSERT_TRUE(ReadWholePath(path, &fresh, &err)) << path << ": errno=" << err; + EXPECT_EQ(Shape(fresh), Shape(chunked)) + << path << ": the chunked read produced a different record shape"; + } +} + +// A fd that already rendered its snapshot must keep draining it after the +// target thread group is gone. Linux `seq_read_iter()` re-enters a handler only +// when the buffer is empty or the position moved, so a continuation read never +// resolves the target again; resolving it there would turn a live snapshot into +// `ESRCH` and cut the record short. +TEST(ProcfsTaskSemantics, SnapshotSurvivesTargetDeath) { + const pid_t child = fork(); + ASSERT_GE(child, 0) << "fork failed: errno=" << errno; + if (child == 0) { + for (;;) { + pause(); + } + } + ReapedChild child_guard(child); + + const std::string path = StatusPath(child, "mountinfo"); + UniqueFd partial(open(path.c_str(), O_RDONLY)); + ASSERT_TRUE(partial.valid()) << "cannot open " << path << ": errno=" << errno; + + // Take one byte first, so the snapshot exists, and only then read the same + // record through a second fd while the target is still alive. That gives the + // expected bytes instead of a byte count that a format change would hide. + char first = 0; + ssize_t taken = 0; + do { + taken = read(partial.get(), &first, 1); + } while (taken < 0 && errno == EINTR); + ASSERT_EQ(1, taken) << "first chunk failed: errno=" << errno; + + std::string whole; + int err = 0; + ASSERT_TRUE(ReadWholePath(path, &whole, &err)) << "cannot read " << path << ": errno=" << err; + ASSERT_GT(whole.size(), 1u) << path << " produced no record"; + + kill(child, SIGKILL); + ASSERT_TRUE(WaitForExit(child)) << "the target was not reaped"; + child_guard.Disarm(); + + // Incidental check that the target really is gone for new readers: the fd + // above is the only thing that may still serve the record. The tid cannot + // be recycled into a live task within this test's lifetime. + UniqueFd after(open(path.c_str(), O_RDONLY)); + EXPECT_FALSE(after.valid()) << "a new open still resolved the reaped task"; + + // ... but this fd still owns its snapshot and drains it byte for byte. + std::string rest; + err = ReadToEof(partial.get(), kReadChunk, &rest); + EXPECT_EQ(0, err) << "continuation read failed: errno=" << err; + std::string reassembled; + reassembled.push_back(first); + reassembled += rest; + EXPECT_EQ(whole, reassembled) << "the frozen snapshot was not drained byte for byte"; +} + +// Reverse guardrail: /proc//oom_score_adj is not a seq_file in Linux +// (snprintf + simple_read_from_buffer), so it must keep serving the tail of a +// freshly rendered record instead of freezing one. Converting it would be a +// regression, not a fix. +TEST(ProcfsTaskSemantics, OomScoreAdjStaysStream) { + const std::string path = StatusPath(getpid(), "oom_score_adj"); + std::string original; + int err = 0; + if (!ReadWholePath(path, &original, &err)) { + GTEST_SKIP() << "cannot read " << path << ": errno=" << err; + } + + UniqueFd fd(open(path.c_str(), O_RDONLY)); + ASSERT_TRUE(fd.valid()) << "cannot open " << path << ": errno=" << errno; + std::string first; + ASSERT_EQ(0, ReadToEof(fd.get(), kReadChunk, &first)); + ASSERT_EQ(original, first); + + const std::string grown = "1000\n"; + UniqueFd wfd(open(path.c_str(), O_WRONLY)); + if (!wfd.valid() || write(wfd.get(), "1000", 4) != 4) { + GTEST_SKIP() << "cannot grow " << path << ": errno=" << errno + << " (this case needs a writable oom_score_adj)"; + } + if (grown.size() <= first.size()) { + // The starting value was already long enough; nothing to observe. + const ssize_t restore = write(wfd.get(), original.c_str(), original.size()); + (void)restore; + GTEST_SKIP() << "oom_score_adj is already " << Escape(original); + } + + char tail[32]; + errno = 0; + const ssize_t n = read(fd.get(), tail, sizeof(tail)); + ASSERT_GE(n, 0) << "read failed: errno=" << errno; + EXPECT_EQ(grown.substr(first.size()), std::string(tail, static_cast(n))) + << "a non-seq_file must serve the tail of the new render, not EOF"; + + // Best effort restore; a non-root caller may be denied the decrease. + const ssize_t restored = write(wfd.get(), original.c_str(), original.size()); + (void)restored; +} + +// --------------------------------------------------------------------------- +// 2. thread-level re-parenting +// --------------------------------------------------------------------------- + +struct ReparentReport { + long original_parent; + long new_parent; + long leader_ppid; + long thread_ppid; + long worker_tid; +}; + +// Publishes the worker's tid over a pipe and then stays alive long enough for +// the owner to observe the thread group. +void* PublishTidWorker(void* arg) { + const int wfd = *static_cast(arg); + prctl(PR_SET_NAME, "worker", 0, 0, 0); + const long tid = GetTid(); + const ssize_t ignored = write(wfd, &tid, sizeof(tid)); + (void)ignored; + for (int i = 0; i < 400; ++i) { + usleep(25000); + } + return nullptr; +} + +// Read a `long` published by PublishTidWorker; -1 when the pipe closed short. +long ReadPublishedTid(int rfd) { + long tid = 0; + size_t got = 0; + while (got < sizeof(tid)) { + const ssize_t n = read(rfd, reinterpret_cast(&tid) + got, sizeof(tid) - got); + if (n <= 0) { + break; + } + got += static_cast(n); + } + return got == sizeof(tid) ? tid : -1; +} + +// When a thread group is re-parented, every thread must report the new parent: +// /proc//task//status used to keep the dead parent while +// /proc//status reported the adopter. +TEST(ProcfsTaskSemantics, ThreadPpidFollowsGroupReparent) { + int pipefd[2]; + ASSERT_EQ(0, pipe(pipefd)) << "pipe failed: errno=" << errno; + + const pid_t top = fork(); + ASSERT_GE(top, 0) << "fork failed: errno=" << errno; + if (top == 0) { + close(pipefd[0]); + const pid_t mid = fork(); + if (mid == 0) { + int tidpipe[2]; + if (pipe(tidpipe) != 0) { + _exit(3); + } + pthread_t th; + if (pthread_create(&th, nullptr, PublishTidWorker, &tidpipe[1]) != 0) { + _exit(3); + } + const long worker_tid = ReadPublishedTid(tidpipe[0]); + close(tidpipe[0]); + close(tidpipe[1]); + if (worker_tid <= 0) { + _exit(3); + } + + ReparentReport rep = {}; + rep.worker_tid = worker_tid; + rep.original_parent = getppid(); + for (int i = 0; i < 2000; ++i) { + if (getppid() != rep.original_parent) { + break; + } + usleep(5000); + } + rep.new_parent = getppid(); + + std::string text; + int read_err = 0; + if (ReadWholePath("/proc/self/status", &text, &read_err)) { + rep.leader_ppid = strtol(Field(ParseStatus(text), "Ppid").c_str(), nullptr, 10); + } else { + rep.leader_ppid = -1; + } + const std::string thread_path = TaskStatusPath(getpid(), rep.worker_tid); + read_err = 0; + if (ReadWholePath(thread_path, &text, &read_err)) { + rep.thread_ppid = strtol(Field(ParseStatus(text), "Ppid").c_str(), nullptr, 10); + } else { + rep.thread_ppid = -read_err; + } + const ssize_t ignored = write(pipefd[1], &rep, sizeof(rep)); + (void)ignored; + _exit(0); + } + // The middle process is the one that dies; its exit is what re-parents + // the worker's whole thread group. + usleep(400000); + _exit(0); + } + + close(pipefd[1]); + ReparentReport rep = {}; + size_t got = 0; + while (got < sizeof(rep)) { + const ssize_t n = read(pipefd[0], reinterpret_cast(&rep) + got, sizeof(rep) - got); + if (n <= 0) { + break; + } + got += static_cast(n); + } + close(pipefd[0]); + int status = 0; + waitpid(top, &status, 0); + + ASSERT_EQ(sizeof(rep), got) << "the re-parented thread group did not report"; + ASSERT_GT(rep.original_parent, 0L); + ASSERT_GT(rep.new_parent, 0L); + EXPECT_NE(rep.original_parent, rep.new_parent) + << "the group was not re-parented, so this case observed nothing"; + EXPECT_EQ(rep.new_parent, rep.leader_ppid); + EXPECT_EQ(rep.leader_ppid, rep.thread_ppid) + << "thread " << rep.worker_tid << " reports Ppid=" << rep.thread_ppid + << " while the group leader reports " << rep.leader_ppid; +} + +// --------------------------------------------------------------------------- +// 3. /proc/ lookup vs. /proc listing +// --------------------------------------------------------------------------- + +// Any live thread can be named by its own tid, and its directory uses the +// thread-group layout (status/stat/statm/limits/cgroup/maps/...). +TEST(ProcfsTaskSemantics, TidDirectoryResolvesForEveryThread) { + const pid_t pid = getpid(); + Worker worker; + ASSERT_TRUE(worker.Start()); + ASSERT_GT(worker.tid(), 0L); + ASSERT_NE(worker.tid(), GetTid()); + + const std::string dir = TidPath(worker.tid()); + const std::vector entries = ListDir(dir); + ASSERT_FALSE(entries.empty()) << "cannot list " << dir << ": errno=" << errno; + for (const char* name : {"status", "stat", "statm", "limits", "cgroup", "maps", "task"}) { + EXPECT_TRUE(Contains(entries, name)) + << dir << " is missing " << name << " (entries: " << entries.size() << ")"; + } + + std::string text; + int err = 0; + ASSERT_TRUE(ReadWholePath(TidPath(worker.tid(), "status"), &text, &err)) + << "cannot read the thread's own directory: errno=" << err; + const auto fields = ParseStatus(text); + EXPECT_EQ(std::to_string(worker.tid()), Field(fields, "Pid")); + EXPECT_EQ(std::to_string(pid), Field(fields, "Tgid")); + EXPECT_EQ("worker", Field(fields, "Name")); + + const std::vector tids = ListDir(TidPath(worker.tid(), "task")); + EXPECT_TRUE(Contains(tids, std::to_string(worker.tid()))); + EXPECT_TRUE(Contains(tids, std::to_string(pid))); +} + +// /proc *lists* group leaders only, even after a non-leader tid has been named +// through /proc/. Linux lists through next_tgid() (PIDTYPE_TGID); the +// per-entry cache must not leak a directory that lookup created from the PID +// link. +TEST(ProcfsTaskSemantics, ProcRootListsOnlyGroupLeaders) { + const pid_t pid = getpid(); + Worker worker; + ASSERT_TRUE(worker.Start()); + ASSERT_GT(worker.tid(), 0L); + + const std::string tid_path = TidPath(worker.tid(), "status"); + UniqueFd fd(open(tid_path.c_str(), O_RDONLY)); + ASSERT_TRUE(fd.valid()) << "cannot open " << tid_path << ": errno=" << errno; + fd.Reset(); + + const std::vector entries = ListDir("/proc"); + ASSERT_FALSE(entries.empty()) << "cannot list /proc: errno=" << errno; + EXPECT_FALSE(Contains(entries, std::to_string(worker.tid()))) + << "/proc leaked the non-leader tid " << worker.tid(); + EXPECT_TRUE(Contains(entries, std::to_string(pid))) + << "/proc dropped the group leader " << pid; + + // The tid is still reachable through the thread-group's task directory. + const std::vector tids = ListDir(StatusPath(pid, "task")); + EXPECT_TRUE(Contains(tids, std::to_string(worker.tid()))); +} + +// A tid from another thread group must not be reachable below /proc//task. +TEST(ProcfsTaskSemantics, ForeignTidUnderTaskIsEnoent) { + const pid_t pid = fork(); + ASSERT_GE(pid, 0) << "fork failed: errno=" << errno; + if (pid == 0) { + for (;;) { + pause(); + } + } + + const std::string path = TaskStatusPath(getpid(), pid); + UniqueFd fd(open(path.c_str(), O_RDONLY)); + const int open_errno = errno; + EXPECT_FALSE(fd.valid()) << path << " resolved to a foreign task"; + EXPECT_EQ(ENOENT, open_errno) << path << ": errno=" << strerror(open_errno); + + kill(pid, SIGKILL); + EXPECT_TRUE(WaitForExit(pid)); +} + +// The issue-reported shape: the group leader exited, a worker is still alive. +// The /proc//task subtree and /proc//status must stay usable, and the +// zombie leader must not be reaped early. +TEST(ProcfsTaskSemantics, TaskSubtreeSurvivesLeaderExit) { + int pipefd[2]; + ASSERT_EQ(0, pipe(pipefd)) << "pipe failed: errno=" << errno; + + const pid_t mid = fork(); + ASSERT_GE(mid, 0) << "fork failed: errno=" << errno; + if (mid == 0) { + close(pipefd[0]); + int tidpipe[2]; + if (pipe(tidpipe) != 0) { + _exit(3); + } + pthread_t th; + if (pthread_create(&th, nullptr, PublishTidWorker, &tidpipe[1]) != 0) { + _exit(3); + } + const long tid = ReadPublishedTid(tidpipe[0]); + close(tidpipe[0]); + close(tidpipe[1]); + const ssize_t ignored = write(pipefd[1], &tid, sizeof(tid)); + (void)ignored; + close(pipefd[1]); + usleep(100000); + // Only the group leader must exit, leaving the group alive. This uses + // exit(2) rather than pthread_exit(): the latter performs a forced + // unwind, which gtest's catch(...) swallows, so the child would fall + // through into the parent's code path instead of exiting the thread. + syscall(SYS_exit, 0); + } + + close(pipefd[1]); + long worker_tid = 0; + size_t got = 0; + while (got < sizeof(worker_tid)) { + const ssize_t n = read(pipefd[0], reinterpret_cast(&worker_tid) + got, + sizeof(worker_tid) - got); + if (n <= 0) { + break; + } + got += static_cast(n); + } + close(pipefd[0]); + ASSERT_EQ(sizeof(worker_tid), got) << "the worker tid was not published"; + usleep(300000); + + const std::vector tids = ListDir(StatusPath(mid, "task")); + EXPECT_EQ(2u, tids.size()) << "the task subtree must list the zombie leader and the worker"; + + std::string text; + int err = 0; + EXPECT_TRUE(ReadWholePath(StatusPath(mid), &text, &err)) + << "the zombie leader's status is unreadable: errno=" << err; + EXPECT_TRUE(ReadWholePath(TaskStatusPath(mid, worker_tid), &text, &err)) + << "the live worker's status is unreadable: errno=" << err; + + int status = 0; + EXPECT_EQ(0, waitpid(mid, &status, WNOHANG)) + << "the group leader was reaped while a worker was still alive"; + + kill(static_cast(worker_tid), SIGKILL); + EXPECT_TRUE(WaitForExit(mid)); +} + +int main(int argc, char** argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/user/apps/tests/dunitest/whitelist.txt b/user/apps/tests/dunitest/whitelist.txt index e6c93ad882..c4587283f7 100644 --- a/user/apps/tests/dunitest/whitelist.txt +++ b/user/apps/tests/dunitest/whitelist.txt @@ -138,3 +138,4 @@ normal/exec_write_access normal/internal_shmem normal/proc_task_status normal/proc_stat_sched_fields +normal/procfs_task_semantics From 906f6fc400e646601880987a4a16fe6f8fbb2da7 Mon Sep 17 00:00:00 2001 From: longjin Date: Tue, 15 Sep 2026 17:13:24 +0000 Subject: [PATCH 2/7] fix(procfs): make seq reads stream per slice and pin the mount view per fd The first version of this fix froze the whole record of every seq-style procfs file per fd. Two review comments on this PR showed where that is too coarse: - /proc//maps buffered a copy of the whole mapping table in every open fd, so a process that creates many VMAs can multiply that buffer by RLIMIT_NOFILE (1048576 by default). Linux freezes one buffer, not the record, so maps now renders one slice at a time from a cursor that is looked up the way find_vma() does (find_nearest(): the mapping covering the address, else the first one above it, as vma_iter_init() plus vma_next() return), and the fd holds at most one page plus the single line that crossed the bound. - /proc//{mounts,mountinfo,mountstats} resolved the target again on the first read, so a setns(), unshare() or chroot() after open() changed what the fd reported. MountView::capture() now pins mnt_ns and the root at open time, the way mounts_open_common() stores p->ns and p->root, and read_at() renders from that view only. The shared driver was aligned with seq_read_iter() in two more places: a read at offset 0 always rewinds, because Linux resets m->index/m->count on every ki_pos == 0 read, and a read that arrives without the state ProcFile::open() installed is refused with EINVAL instead of silently falling back to the per-read rendering this fix removes. Tests: normal/procfs_task_semantics grows to 18 cases, including MapsStreamsMappingsAddedAfterFirstRead, MapsLargeReadReassemblesTheSameRecord, MountInfoKeepsTheRootPinnedAtOpen, MapsStreamKeepsCopiedBytesWhenTargetDies and MapsStreamKeepsTheAddressSpaceOpenedOn. Restoring the whole-table maps render, or the read-time mount resolution, makes 5 of the 18 fail, so they are falsifiable rather than tautological. Guest run (QEMU/KVM): 18/18, and the 11 existing procfs suites are unchanged from the baseline (proc_thread_accounting_test still fails for a missing /sys/fs/cgroup in the probe rootfs, on the baseline as well). Signed-off-by: longjin --- kernel/src/filesystem/procfs/mod.rs | 29 +- kernel/src/filesystem/procfs/mount/collect.rs | 14 +- .../procfs/mount/inode/pid_mount.rs | 46 +- kernel/src/filesystem/procfs/mount/mod.rs | 4 +- kernel/src/filesystem/procfs/mount/render.rs | 23 +- kernel/src/filesystem/procfs/mount/view.rs | 57 +++ kernel/src/filesystem/procfs/pid/id_map.rs | 2 +- kernel/src/filesystem/procfs/pid/maps.rs | 146 ++++-- kernel/src/filesystem/procfs/utils.rs | 234 +++++++-- kernel/src/mm/ucontext/mappings.rs | 2 - .../suites/normal/procfs_task_semantics.cc | 462 ++++++++++++++++++ 11 files changed, 906 insertions(+), 113 deletions(-) create mode 100644 kernel/src/filesystem/procfs/mount/view.rs diff --git a/kernel/src/filesystem/procfs/mod.rs b/kernel/src/filesystem/procfs/mod.rs index 298ce843c6..aba058b4ef 100644 --- a/kernel/src/filesystem/procfs/mod.rs +++ b/kernel/src/filesystem/procfs/mod.rs @@ -3,7 +3,7 @@ //! 实现 Linux 兼容的 /proc 文件系统 use crate::mm::ucontext::AddressSpace; -use alloc::{sync::Arc, vec::Vec}; +use alloc::sync::Arc; use system_error::SystemError; use crate::{ @@ -13,6 +13,7 @@ use crate::{ use super::vfs::mount::MountFlags; use super::vfs::InodeMode; +use mount::MountView; mod cmdline; mod cpuinfo; @@ -50,23 +51,33 @@ pub(super) use template::Builder; /// procfs 文件私有数据 #[derive(Debug, Clone)] pub struct ProcfsFilePrivateData { - pub data: Vec, pub open_cred: Arc, - pub pinned_vm: Option>, - /// Continuation position for seq-style files (mirrors Linux `seq_file::m->read_pos`). + /// Address space this fd was opened on, taken by `open()` of the files that + /// address one (`/proc/[pid]/mem`, `/proc/[pid]/maps`). /// - /// `None` means this fd has not rendered yet; `Some(p)` means the snapshot is - /// ready and the next read continues at `p`. See `utils::proc_read_snapshot()`. - pub read_pos: Option, + /// The descriptor, deliberately not the memory: Linux `proc_mem_open()` + /// grabs the `mm_struct` and then drops the user reference again + /// ("but do not pin its memory"), so an `execve()` or an exit in the target + /// still tears the mappings down while this fd stays open. Each read + /// re-checks the user count (`mmget_not_zero()`), which is how both files + /// learn that there is nothing left to serve. + pub pinned_vm: Option>, + /// Streaming state of a seq-style record (Linux `struct seq_file`). Only + /// `utils::proc_read_seq()` and `utils::proc_read_snapshot()` touch it. + pub(crate) seq: utils::ProcfsSeq, + /// Mount namespace and root directory pinned by `open()` for + /// `/proc/[pid]/{mounts,mountinfo,mountstats}`, as `mounts_open_common()` + /// does; `None` for every other procfs file. + pub(crate) mount_view: Option, } impl ProcfsFilePrivateData { pub fn new() -> Self { ProcfsFilePrivateData { - data: Vec::new(), open_cred: ProcessManager::current_pcb().cred(), pinned_vm: None, - read_pos: None, + seq: utils::ProcfsSeq::default(), + mount_view: None, } } } diff --git a/kernel/src/filesystem/procfs/mount/collect.rs b/kernel/src/filesystem/procfs/mount/collect.rs index 4826e19c1e..f96255faf9 100644 --- a/kernel/src/filesystem/procfs/mount/collect.rs +++ b/kernel/src/filesystem/procfs/mount/collect.rs @@ -8,9 +8,10 @@ use crate::{ FileSystem, MountFS, }, libs::casting::DowncastArc, - process::ProcessControlBlock, }; +use super::MountView; + #[derive(Debug)] pub(crate) struct ProcMountEntry { pub mount: Arc, @@ -26,20 +27,15 @@ pub(crate) struct ProcMountEntry { } pub(crate) fn collect_visible_mounts( - target: &Arc, + view: &MountView, ) -> Result<(Vec, String), SystemError> { - let root = target - .try_fs_struct() - .ok_or(SystemError::ESRCH)? - .root() - .downcast_arc::() - .ok_or(SystemError::EINVAL)?; + let root = view.root.clone(); if root.is_disconnected() { return Ok((Vec::new(), "/".to_string())); } let root_mount = root.mount_fs(); - let mount_namespace = target.nsproxy().mnt_ns.clone(); + let mount_namespace = view.ns.clone(); let mut mounts = Vec::new(); let mount_root = root_mount .root_inode() diff --git a/kernel/src/filesystem/procfs/mount/inode/pid_mount.rs b/kernel/src/filesystem/procfs/mount/inode/pid_mount.rs index 81f56a5028..fb1faa9d46 100644 --- a/kernel/src/filesystem/procfs/mount/inode/pid_mount.rs +++ b/kernel/src/filesystem/procfs/mount/inode/pid_mount.rs @@ -2,7 +2,7 @@ use core::fmt::Debug; use crate::filesystem::{ procfs::{ - mount::{render_mount_file_for_task, ProcMountRenderKind}, + mount::{render_mount_file, MountView, ProcMountRenderKind}, pid::ProcPidTarget, template::{Builder, FileOps, ProcFileBuilder}, utils::proc_read_snapshot, @@ -42,11 +42,21 @@ impl FileOps for MountProcFileOps { self.target.owner_uid_gid() } - fn open(&self, _data: &mut MutexGuard) -> Result<(), SystemError> { - // Linux `mounts_open_common()` resolves the target with `get_proc_task()` - // at open time and fails with `ESRCH` when it is already gone. The record - // itself is rendered on the first read, like any other `seq_file`. - self.target.thread_group_leader().ok_or(SystemError::ESRCH)?; + fn open(&self, data: &mut MutexGuard) -> Result<(), SystemError> { + // Linux `mounts_open_common()` resolves the target once at open time and + // keeps its mount namespace and root path in the seq private data, so a + // `setns()`, `unshare()` or `chroot()` performed afterwards cannot + // change what this fd reports. The record itself is rendered on the + // first read, like any other `seq_file`. + let task = self + .target + .thread_group_leader() + .ok_or(SystemError::ESRCH)?; + let view = MountView::capture(&task)?; + let FilePrivateData::Procfs(pdata) = &mut **data else { + return Err(SystemError::EINVAL); + }; + pdata.mount_view = Some(view); Ok(()) } @@ -57,16 +67,20 @@ impl FileOps for MountProcFileOps { buf: &mut [u8], mut data: MutexGuard, ) -> Result { - // The target is resolved by the renderer, never on a continuation read. - // `seq_read_iter()` does not re-enter a handler while its buffer still - // holds data, so a reader that already took the first chunk keeps - // draining this fd's snapshot even after the thread group is gone. - proc_read_snapshot(offset, len, buf, &mut data, || { - let task = self - .target - .thread_group_leader() - .ok_or(SystemError::ESRCH)?; - render_mount_file_for_task(&task, self.kind) + // The view is taken once, by `open()`: rendering resolves neither the + // target nor its root again, so a reader that already took the first + // chunk keeps draining this fd's record even after the thread group is + // gone, and a root change after open() does not move an open fd. A read + // without that state did not come through the procfs `open()` hook and + // has no view to render from. + let view = { + let FilePrivateData::Procfs(pdata) = &*data else { + return Err(SystemError::EINVAL); + }; + pdata.mount_view.clone().ok_or(SystemError::EINVAL)? + }; + proc_read_snapshot(offset, len, buf, &mut data, move || { + render_mount_file(&view, self.kind) }) } } diff --git a/kernel/src/filesystem/procfs/mount/mod.rs b/kernel/src/filesystem/procfs/mount/mod.rs index 2dbd678d9e..e70e26951e 100644 --- a/kernel/src/filesystem/procfs/mount/mod.rs +++ b/kernel/src/filesystem/procfs/mount/mod.rs @@ -7,5 +7,7 @@ mod fields; pub(crate) mod format; pub(crate) mod inode; mod render; +mod view; -pub(crate) use render::{render_mount_file_for_task, ProcMountRenderKind}; +pub(crate) use render::{render_mount_file, ProcMountRenderKind}; +pub(crate) use view::MountView; diff --git a/kernel/src/filesystem/procfs/mount/render.rs b/kernel/src/filesystem/procfs/mount/render.rs index 40fa8af1fb..d0b77dc82c 100644 --- a/kernel/src/filesystem/procfs/mount/render.rs +++ b/kernel/src/filesystem/procfs/mount/render.rs @@ -1,16 +1,14 @@ -use alloc::{string::String, sync::Arc, vec::Vec}; +use alloc::{string::String, vec::Vec}; use system_error::SystemError; -use crate::{ - filesystem::vfs::mount::with_topology_snapshot, - process::ProcessControlBlock, -}; +use crate::filesystem::vfs::mount::with_topology_snapshot; use super::{ collect::collect_visible_mounts, fields::MountProcFields, format::{mountinfo_line, mounts_line, mountstats_line}, + MountView, }; #[derive(Clone, Copy, Debug)] @@ -20,17 +18,18 @@ pub(crate) enum ProcMountRenderKind { MountStats, } -/// Render one mount-family record (`mounts` / `mountinfo` / `mountstats`) for -/// `target`. +/// Renders one mount-family record (`mounts` / `mountinfo` / `mountstats`) from +/// `view`. /// /// Linux serves these through `seq_open_private()` (`fs/proc_namespace.c`): the -/// record is produced by the reader, not at open time, so a file that is opened -/// and read much later shows the topology of the moment it is read. -pub(crate) fn render_mount_file_for_task( - target: &Arc, +/// record is produced by the reader, not at open time, so a file opened and read +/// much later shows the topology the namespace reached by the time it is read — +/// within the namespace and root directory `mounts_open_common()` pinned at open. +pub(crate) fn render_mount_file( + view: &MountView, kind: ProcMountRenderKind, ) -> Result, SystemError> { - let (entries, _root_path) = with_topology_snapshot(|| collect_visible_mounts(target))?; + let (entries, _root_path) = with_topology_snapshot(|| collect_visible_mounts(view))?; let mut rendered = String::new(); for entry in &entries { diff --git a/kernel/src/filesystem/procfs/mount/view.rs b/kernel/src/filesystem/procfs/mount/view.rs new file mode 100644 index 0000000000..8583b12367 --- /dev/null +++ b/kernel/src/filesystem/procfs/mount/view.rs @@ -0,0 +1,57 @@ +use core::fmt::Debug; + +use alloc::sync::Arc; + +use system_error::SystemError; + +use crate::{ + filesystem::vfs::mount::MountFSInode, + libs::casting::DowncastArc, + process::{namespace::mnt::MntNamespace, ProcessControlBlock}, +}; + +/// The mount namespace and root directory a `/proc/[pid]/{mounts,mountinfo, +/// mountstats}` fd renders from. +/// +/// Linux `mounts_open_common()` resolves the target once at open time: under +/// `task_lock()` it takes `task->nsproxy->mnt_ns` (`get_mnt_ns()`) and +/// `get_fs_root(task->fs, &root)`, and stores them in the seq private data +/// (`p->ns`, `p->root`, the path `seq_path_root()` renders from). A `setns()`, +/// `unshare()` or `chroot()` performed afterwards therefore cannot change what +/// an already open fd reports. +#[derive(Clone)] +pub(crate) struct MountView { + /// Mount namespace the record is collected from. + pub ns: Arc, + /// Root directory of the target, used for path rendering the way + /// `seq_path_root()` uses `proc_mounts::root`. + pub root: Arc, +} + +impl Debug for MountView { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + f.debug_struct("MountView") + .field("ns", &Arc::as_ptr(&self.ns)) + .field("root", &self.root) + .finish() + } +} + +impl MountView { + /// Pins the view of `task`, whose thread group leader the caller resolved + /// already (`mounts_open_common()` reports `ESRCH` before it gets here). + /// + /// The failures follow that function: a task without a root directory is + /// `ENOENT`, like the `!task->fs` check, and a root that is not a mount is + /// `EINVAL`, like its invalid-`root` check. + pub(crate) fn capture(task: &Arc) -> Result { + let ns = task.nsproxy().mnt_ns.clone(); + let root = task + .try_fs_struct() + .ok_or(SystemError::ENOENT)? + .root() + .downcast_arc::() + .ok_or(SystemError::EINVAL)?; + Ok(Self { ns, root }) + } +} diff --git a/kernel/src/filesystem/procfs/pid/id_map.rs b/kernel/src/filesystem/procfs/pid/id_map.rs index 08bb0a1917..f2a86cebd1 100644 --- a/kernel/src/filesystem/procfs/pid/id_map.rs +++ b/kernel/src/filesystem/procfs/pid/id_map.rs @@ -384,7 +384,7 @@ impl FileOps for IdMapFileOps { let ctx = IdMapWriteContext { map_type: self.map_type, target_ns: user_ns.clone(), - opener_cred, + opener_cred: opener_cred.clone(), target_owner: inner.owner, target_flags: inner.flags, target_parent_could_setfcap: inner.parent_could_setfcap, diff --git a/kernel/src/filesystem/procfs/pid/maps.rs b/kernel/src/filesystem/procfs/pid/maps.rs index 96d99578f8..acb3213368 100644 --- a/kernel/src/filesystem/procfs/pid/maps.rs +++ b/kernel/src/filesystem/procfs/pid/maps.rs @@ -9,11 +9,14 @@ use crate::{ procfs::{ pid::ProcPidTarget, template::{Builder, FileOps, ProcFileBuilder}, - utils::proc_read_snapshot, + utils::proc_read_seq, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }, - mm::{ucontext::LockedVMA, MemoryManagementArch, VmFlags}, + mm::{ + ucontext::{AddressSpace, LockedVMA}, + MemoryManagementArch, VirtAddr, VmFlags, + }, }; use alloc::{ format, @@ -115,31 +118,90 @@ fn format_dev_inode_and_path( (String::from("00:00 0"), String::new()) } -/// 生成 /proc/[pid]/maps 内容 -fn generate_maps_content(target: &ProcPidTarget) -> Result, SystemError> { +/// Renders the mappings from `cursor` on, the way Linux `m_start()` / `m_next()` +/// (`fs/proc/task_mmu.c`) walk the mapping table, and stops once `budget` bytes +/// are out. +/// +/// `cursor` is the address the reader stopped at, which is exactly what +/// `m_start()` keeps in `*ppos` (`proc_get_vma()` stores `vma->vm_start`: the +/// address of the mapping the reader has *not* seen yet). It is looked up the +/// way `vma_next()` does, so the mapping this slice starts with is the one +/// covering `cursor`, and a reader never sees a mapping twice. Returns the +/// address the next slice resumes at, or `None` when the table ends here. +/// +/// Rendering one slice at a time is what keeps an open fd from buffering a copy +/// of the whole mapping table: Linux `seq_file` likewise holds one buffer, not +/// the record. `budget` bounds the slice and is capped a page at a time by +/// [`proc_read_seq()`], so an fd holds at most one page of the table plus the +/// one line that crossed the bound. +fn render_maps_slice( + target: &ProcPidTarget, + vm: Option<&Arc>, + cursor: Option, + budget: usize, + out: &mut Vec, +) -> Result, SystemError> { + // Linux `m_start()` resolves the task for every record it produces, so a + // target that is gone ends the stream with `-ESRCH` even though the address + // space itself is pinned by `open()`. let target_pcb = target.thread_group_leader().ok_or(SystemError::ESRCH)?; - let vm = { - let basic = target_pcb.basic(); - basic.user_vm() - }; + // A task without an address space (kernel thread) has no record to render. let Some(vm) = vm else { - return Ok(Vec::new()); + return Ok(None); + }; + // Linux `m_start()`: `mm = priv->mm; if (!mm || !mmget_not_zero(mm)) return + // NULL;`. The descriptor `open()` took can outlive the memory it described, + // so the user count is what says whether this fd still has a table to walk; + // `m_start()`/`m_stop()` hold the same reference across one record. The + // stream therefore ends where the address space ends, the way it does when + // the target exits or execs away from it. + let Some(_mm_user) = vm.try_acquire() else { + return Ok(None); }; let Some(fs) = target_pcb.try_fs_struct() else { - return Ok(Vec::new()); + return Ok(None); }; let root_prefix = fs.root().absolute_path().unwrap_or_default(); let as_guard = vm.read_guard_no_reservations(); - // 收集并按地址排序 - let mut vmas: Vec> = as_guard.mappings.iter_vmas().cloned().collect(); - vmas.sort_by_key(|v| v.lock().region().start().data()); + // Linux resumes through `find_vma()`: `vma_iter_init(mm, last_addr)` and + // `vma_next()` hand back the mapping that *covers* the resume address, or + // the first mapping above it. `find_nearest()` looks the address up the same + // way, so a mapping that still covers the cursor keeps its place in the + // stream even if it was moved below the cursor since the previous slice. + // The walk itself only moves upwards, so it costs O(log n) per slice, not a + // rescan. + let resume_addr = VirtAddr::new(cursor.unwrap_or(0)); + let covering = as_guard.mappings.find_nearest(resume_addr); + let walk_from = covering + .as_ref() + .map(|vma| VirtAddr::new(vma.lock().region().start().data() + 1)) + .unwrap_or(resume_addr); - let mut out: Vec = Vec::new(); + let mut resume = None; + for vma in covering + .into_iter() + .chain(as_guard.mappings.iter_vmas_starting_at(walk_from)) + { + if !out.is_empty() && out.len() >= budget { + resume = Some(vma.lock().region().start().data()); + break; + } + append_map_line(&vma, &root_prefix, out); + } + + if out.is_empty() && cursor.is_none() { + // 确保文件以换行符结尾 + out.extend_from_slice(b"\n"); + } + Ok(resume) +} - for vma in vmas { +/// Appends one line of the mapping table, as Linux `show_map_vma()` writes it. +fn append_map_line(vma: &Arc, root_prefix: &str, out: &mut Vec) { + { let g = vma.lock(); let region = *g.region(); let vm_flags = *g.vm_flags(); @@ -152,9 +214,9 @@ fn generate_maps_content(target: &ProcPidTarget) -> Result, SystemError> let (dev_ino, path_tail) = if let Some(f) = g.vm_file() { let inode = f.inode(); - format_dev_inode_and_path(Some(&inode), &root_prefix) + format_dev_inode_and_path(Some(&inode), root_prefix) } else { - format_dev_inode_and_path(None, &root_prefix) + format_dev_inode_and_path(None, root_prefix) }; let line = format!( @@ -171,16 +233,34 @@ fn generate_maps_content(target: &ProcPidTarget) -> Result, SystemError> ); out.extend_from_slice(line.as_bytes()); } - - // 确保文件以换行符结尾 - if out.is_empty() { - out.extend_from_slice(b"\n"); - } - - Ok(out) } impl FileOps for MapsFileOps { + fn open(&self, data: &mut MutexGuard) -> Result<(), SystemError> { + // Linux `proc_maps_open()` -> `proc_mem_open()`: the target's address + // space is taken under the exec lock, so this fd is bound to the space + // it was opened on and an `execve()` between two reads cannot move the + // stream into a new one. The descriptor is held, not the memory (Linux + // `mmgrab()` then `mmput()`, "but do not pin its memory"), so the + // mappings still go away when the target leaves them behind; each slice + // re-checks the user count for that, as `m_start()` does. + let task = self + .target + .thread_group_leader() + .ok_or(SystemError::ESRCH)?; + let pinned = { + let _exec_guard = task.exec_update_read(); + task.basic().user_vm() + }; + let FilePrivateData::Procfs(pdata) = &mut **data else { + return Err(SystemError::EINVAL); + }; + // `None` is a task without an address space (a kernel thread): Linux + // `m_start()` renders no record for it either. + pdata.pinned_vm = pinned; + Ok(()) + } + fn read_at( &self, offset: usize, @@ -188,10 +268,20 @@ impl FileOps for MapsFileOps { buf: &mut [u8], mut data: MutexGuard, ) -> Result { - // One fd sees one snapshot of the whole mapping table, so a mapping that - // appears while the reader is mid-stream cannot tear the byte stream. - proc_read_snapshot(offset, len, buf, &mut data, || { - generate_maps_content(&self.target) + // The table to stream comes from the address space `open()` took; a + // continuation read must not resolve it again. A read that arrives + // without that state did not come through the procfs `open()` hook and + // has no address space to stream: reporting an empty record for it would + // claim the target maps nothing. + let FilePrivateData::Procfs(pdata) = &*data else { + return Err(SystemError::EINVAL); + }; + let vm = pdata.pinned_vm.clone(); + // One mapping is rendered per slice, so an fd holds at most one line of + // the table no matter how many mappings the target has; the cursor keeps + // a mapping that appears mid-stream from tearing the byte stream. + proc_read_seq(offset, len, buf, &mut data, |cursor, budget, out| { + render_maps_slice(&self.target, vm.as_ref(), cursor, budget, out) }) } } diff --git a/kernel/src/filesystem/procfs/utils.rs b/kernel/src/filesystem/procfs/utils.rs index 8953eb437a..56240cc3e7 100644 --- a/kernel/src/filesystem/procfs/utils.rs +++ b/kernel/src/filesystem/procfs/utils.rs @@ -1,7 +1,10 @@ use alloc::vec::Vec; use system_error::SystemError; -use crate::{filesystem::vfs::FilePrivateData, libs::mutex::MutexGuard}; +use crate::{ + arch::MMArch, filesystem::vfs::FilePrivateData, libs::mutex::MutexGuard, + mm::MemoryManagementArch, +}; /// 去除Vec中所有的\0,并在结尾添加\0 #[inline] @@ -31,56 +34,217 @@ pub(super) fn proc_read( return Ok(src.len()); } -/// Snapshot read for procfs: mirrors Linux `seq_file` (`single_open()` / -/// `seq_read_iter()` / `seq_lseek()`). +/// State of a seq-style procfs fd (Linux `struct seq_file`). /// -/// One fd re-renders only on the first read, or when the read position no longer -/// matches the continuation position: +/// The fd keeps the bytes a record source produced but the reader has not +/// drained yet, where those bytes sit in the file, and the cursor the source +/// resumes from. See [`proc_read_seq()`]. +#[derive(Debug, Clone, Default)] +pub(crate) struct ProcfsSeq { + /// Slice produced by the record source that the reader has not drained. + rendered: Vec, + /// How many bytes of `rendered` the reader already copied out. + taken: usize, + /// File offset that `rendered[taken]` maps to. + offset: usize, + /// Where the record source resumes; `None` before its first slice. + cursor: Option, + /// The source reported the end of the record. + exhausted: bool, +} + +impl ProcfsSeq { + /// Bytes of the current slice that are still to be copied out. + fn available(&self) -> usize { + self.rendered.len() - self.taken + } + + /// Drops the buffered slice and the cursor, so the next slice starts over at + /// the beginning of the record (Linux `traverse()`). + fn reset(&mut self) { + // Replacing the buffer, rather than clearing it, releases a slice that + // had grown large before the rewind. + self.rendered = Vec::new(); + self.taken = 0; + self.offset = 0; + self.cursor = None; + self.exhausted = false; + } + + /// Copies the buffered bytes out and advances the file position. + fn take_into(&mut self, buf: &mut [u8]) { + let end = self.taken + buf.len(); + buf.copy_from_slice(&self.rendered[self.taken..end]); + self.taken = end; + self.offset += buf.len(); + } + + /// Drops buffered bytes without copying them out, for a seek over bytes the + /// reader never asked for. + fn discard(&mut self, count: usize) { + self.taken += count; + self.offset += count; + } +} + +/// Most bytes one slice of a record may render into an fd's buffer. /// -/// - `offset == read_pos` and `offset != 0`: continue from the snapshot, so -/// **content growing between two reads does not revive EOF**; -/// - `offset == 0`: re-render, matching `seq_read_iter()`'s `ki_pos == 0` reset; -/// - anything else: re-render and reposition at `offset`, matching `traverse()`. +/// Linux `seq_file` starts with one page (`m->size = PAGE_SIZE`) and only grows +/// the buffer for a single record that cannot fit in it +/// (`fs/seq_file.c:seq_read_iter()`), so a reader's read length does not decide +/// how much a seq file buffers. Bounding a slice the same way keeps a large read +/// from turning into a large per-fd buffer for a record source that can render +/// arbitrarily much, such as the mapping table of `/proc/[pid]/maps`. +const SEQ_SLICE_MAX: usize = MMArch::PAGE_SIZE; + +/// Serves one procfs record the way Linux `seq_read_iter()` does. /// -/// `render` is only called when needed (the continuation path never re-renders). -/// A failed render drops the continuation point, the way a failed `traverse()` -/// resets the buffer (`fs/seq_file.c:196-203`), so the next read re-renders -/// instead of serving a stale snapshot. -pub(super) fn proc_read_snapshot( +/// The fd holds the bytes of the slice the source produced plus an opaque resume +/// cursor, and the source is entered only when that buffer runs dry. A record +/// that changes in between therefore cannot tear the byte stream, and once the +/// source reports the end of the record the fd keeps reporting EOF. +/// +/// `source` is handed the cursor of the previous slice (`None` for the first +/// slice of a record), how many bytes the reader still wants, and an empty +/// buffer to render into. The wanted byte count is an upper bound on the slice, +/// never more than [`SEQ_SLICE_MAX`], so an incremental source may have to be +/// entered several times before one read is satisfied. It returns the cursor to +/// resume from, or `None` when the record ends after this slice. +/// +/// Position rules mirror `seq_read_iter()`/`seq_lseek()`: +/// - `offset == 0`: rewind, so the record is rendered again; +/// - `offset == seq.offset`: continue; the source is not entered while buffered +/// bytes remain, even when the target is already gone; +/// - any other offset: seek, so the record is rendered again from the start and +/// the first `offset` bytes are dropped. +/// - a seek past the end of the record parks the fd at the requested offset, so +/// reading there reports EOF instead of rendering the record again. +/// +/// An empty request (`len == 0`, or a full buffer) returns 0 without touching +/// the fd, and a source that fails after this call already copied bytes out +/// still reports those bytes, as `seq_read_iter()` returns `copied` and discards +/// the error once it copied something. +pub(super) fn proc_read_seq( offset: usize, len: usize, buf: &mut [u8], data: &mut MutexGuard, - render: F, + mut source: F, ) -> Result where - F: FnOnce() -> Result, SystemError>, + F: FnMut(Option, usize, &mut Vec) -> Result, SystemError>, { + // A read that reaches `read_at()` without the state `ProcFile::open()` left + // behind is not a procfs read. Serving it from state that lives no longer + // than this call would render and slice the record per read, which is the + // tear this driver exists to prevent, so it is refused instead. let FilePrivateData::Procfs(pdata) = &mut **data else { - // A few callers (e.g. symlink reads) reach read_at() without procfs - // private data. Fall back to render-per-read, i.e. the previous behaviour. - let content = render()?; - return proc_read(offset, len, buf, &content); + return Err(SystemError::EINVAL); }; + let seq = &mut pdata.seq; - let pos = match pdata.read_pos { - Some(pos) if pos == offset && offset != 0 => pos, - _ => { - match render() { - Ok(rendered) => { - pdata.data = rendered; - pdata.read_pos = Some(offset); - } + // `seq_read_iter()` returns before it touches the iterator when the request + // is empty, and `copy_to_iter()` never copies more than the iovec holds. + let len = len.min(buf.len()); + if len == 0 { + return Ok(0); + } + + let mut skip = 0; + // A read at offset 0 rewinds even when the fd already sits there, because + // `seq_read_iter()` resets `m->index`/`m->count` on every `ki_pos == 0` + // read: a record that was empty when the fd was first read has to be + // rendered again rather than replaying that EOF forever. + if offset == 0 || seq.offset != offset { + seq.reset(); + skip = offset; + } + + let mut written = 0; + loop { + if written >= len { + // The reader is satisfied, and `seq_read_iter()` likewise stops + // filling once the iterator has no room left. + break; + } + if seq.available() == 0 { + if seq.exhausted { + break; + } + seq.rendered.clear(); + seq.taken = 0; + let want = (len - written).min(SEQ_SLICE_MAX); + let resume = match source(seq.cursor, want, &mut seq.rendered) { + Ok(resume) => resume, Err(err) => { - pdata.read_pos = None; - return Err(err); + // A slice that could not be produced is not part of the + // record: drop whatever it wrote and keep the cursor, so a + // later read retries that record. Bytes this call already + // copied out still count, the way `seq_read_iter()` returns + // `copied` and drops `err` once it copied something. + seq.rendered.clear(); + seq.taken = 0; + if written == 0 { + return Err(err); + } + return Ok(written); } + }; + seq.cursor = resume; + // A source that produced nothing cannot make progress, so its slice + // is treated as the end of the record instead of being re-entered. + seq.exhausted = resume.is_none() || seq.rendered.is_empty(); + if seq.available() == 0 { + break; } - offset } - }; - let n = proc_read(pos, len, buf, &pdata.data)?; - pdata.read_pos = Some(pos + n); - Ok(n) + if skip > 0 { + let dropped = skip.min(seq.available()); + seq.discard(dropped); + skip -= dropped; + continue; + } + + // `available() > 0` and `written < len <= buf.len()`, so this slice is + // in bounds and never empty. + let take = seq.available().min(len - written); + seq.take_into(&mut buf[written..written + take]); + written += take; + } + + if skip > 0 { + // The seek ran off the end of the record, so no byte of it is pending. + // Linux `traverse()` still records `m->read_pos = iocb->ki_pos`, which is + // what keeps a later read at that position from rendering again. + seq.offset = offset; + } + return Ok(written); +} + +/// Serves a procfs record that a file renders in one piece, the way Linux +/// `single_open()` files are read. +/// +/// `render` is entered on the first read of an fd and again only when that fd is +/// rewound or seeked. While the fd still holds bytes of its record, a later +/// `read()` replays them, so content that grows in between neither tears the +/// byte stream nor revives EOF. +pub(super) fn proc_read_snapshot( + offset: usize, + len: usize, + buf: &mut [u8], + data: &mut MutexGuard, + mut render: F, +) -> Result +where + F: FnMut() -> Result, SystemError>, +{ + proc_read_seq(offset, len, buf, data, move |cursor, _want, out| { + // `None` is the start of a record: first read, rewind or seek. Any other + // cursor means the whole record already went out in the first slice. + if cursor.is_none() { + *out = render()?; + } + Ok(None) + }) } diff --git a/kernel/src/mm/ucontext/mappings.rs b/kernel/src/mm/ucontext/mappings.rs index a786ceb9d3..f311eafba2 100644 --- a/kernel/src/mm/ucontext/mappings.rs +++ b/kernel/src/mm/ucontext/mappings.rs @@ -67,7 +67,6 @@ impl UserMappings { /// Check whether any VMA in the current process contains the specified virtual address. /// /// Returns the Arc pointer of the VMA containing the address if found, otherwise returns None. - #[allow(dead_code)] pub fn contains(&self, vaddr: VirtAddr) -> Option> { let (_, vma) = self.vmas_by_start.range(..=vaddr).next_back()?; if vma.lock().region.contains(vaddr) { @@ -85,7 +84,6 @@ impl UserMappings { /// ## Returns /// - Some(Arc): The VMA containing the address or the nearest subsequent VMA /// - None: No VMA found - #[allow(dead_code)] pub fn find_nearest(&self, vaddr: VirtAddr) -> Option> { if let Some(vma) = self.contains(vaddr) { return Some(vma); diff --git a/user/apps/tests/dunitest/suites/normal/procfs_task_semantics.cc b/user/apps/tests/dunitest/suites/normal/procfs_task_semantics.cc index 98d0b50586..bd3eb5f294 100644 --- a/user/apps/tests/dunitest/suites/normal/procfs_task_semantics.cc +++ b/user/apps/tests/dunitest/suites/normal/procfs_task_semantics.cc @@ -26,8 +26,11 @@ #include #include #include +#include #include +#include #include +#include #include #include #include @@ -44,6 +47,12 @@ namespace { constexpr size_t kReadChunk = 128; constexpr int kPollTimeoutMs = 2000; +/// Argument that turns the test binary into the exec()ed helper of +/// MapsStreamKeepsTheAddressSpaceOpenedOn: it maps a marker region, reports its +/// address on the inherited pipe and parks, so the parent can watch the target +/// change address spaces while an fd streams /proc//maps. +constexpr char kMapsExecParkArg[] = "procfs_task_semantics_maps_exec_park"; + class UniqueFd { public: UniqueFd() = default; @@ -340,6 +349,151 @@ class Worker { bool started_ = false; }; + +// Reads one line with 1-byte reads, leaving the fd on a line boundary. +bool ReadLine(int fd, std::string* line) { + line->clear(); + for (;;) { + char c = 0; + const ssize_t n = read(fd, &c, 1); + if (n < 0) { + if (errno == EINTR) { + continue; + } + return false; + } + if (n == 0) { + return !line->empty(); + } + line->push_back(c); + if (c == '\n') { + return true; + } + if (line->size() > 4096) { + return false; + } + } +} + +// True when a mapping line of `maps` covers `addr`. Ranges are compared instead +// of whole lines so that merging with a neighbouring mapping cannot make the +// check miss. +bool MapsCover(const std::string& maps, unsigned long addr) { + size_t pos = 0; + while (pos < maps.size()) { + const size_t nl = maps.find('\n', pos); + const std::string line = + (nl == std::string::npos) ? maps.substr(pos) : maps.substr(pos, nl - pos); + unsigned long start = 0; + unsigned long end = 0; + if (sscanf(line.c_str(), "%lx-%lx", &start, &end) == 2 && start <= addr && addr < end) { + return true; + } + if (nl == std::string::npos) { + break; + } + pos = nl + 1; + } + return false; +} + +// chroot() into a directory that exists or can be created here. Returns false +// when the environment offers none, so the caller can skip instead of failing. +bool ChrootAway() { + const char* const kCandidates[] = {"/dunitest-chroot", "/tmp", "/dev"}; + for (const char* dir : kCandidates) { + if (mkdir(dir, 0755) != 0 && errno != EEXIST) { + continue; + } + if (chroot(dir) == 0) { + return true; + } + } + return false; +} + +// Runs in a forked child, so chroot() cannot disturb the suite. Returns 0 when +// the fd kept the view it was opened with, 1 when the environment cannot show +// the difference, and 2 when the fd followed the new root. +int CheckPinnedMountView() { + UniqueFd fd(open("/proc/self/mountinfo", O_RDONLY)); + if (!fd.valid()) { + return 1; + } + std::string before; + if (ReadToEof(fd.get(), kReadChunk, &before) != 0 || before.empty()) { + return 1; + } + + if (!ChrootAway()) { + return 1; + } + // Evidence that the root really changed: /proc is no longer reachable. + UniqueFd unreachable(open("/proc/self/mountinfo", O_RDONLY)); + if (unreachable.valid()) { + return 1; + } + + // Rewinding forces a re-render: a fd that resolved the target again would + // render from the new root instead of the one pinned at open. + if (lseek(fd.get(), 0, SEEK_SET) != 0) { + return 1; + } + std::string after; + if (ReadToEof(fd.get(), kReadChunk, &after) != 0) { + return 1; + } + return after == before ? 0 : 2; +} + +// Reads one byte, retrying on EINTR. Returns -1 with errno set when the read +// fails, 0 at EOF and 1 when a byte was stored. +int ReadByte(int fd, char* out) { + for (;;) { + const ssize_t n = read(fd, out, 1); + if (n < 0 && errno == EINTR) { + continue; + } + return static_cast(n); + } +} + +// Fixed-size payload exchanged with the exec()ed helper over a pipe. +bool WriteRaw(int fd, const void* data, size_t len) { + const char* p = static_cast(data); + size_t done = 0; + while (done < len) { + const ssize_t n = write(fd, p + done, len - done); + if (n < 0) { + if (errno == EINTR) { + continue; + } + return false; + } + done += static_cast(n); + } + return true; +} + +bool ReadRaw(int fd, void* data, size_t len) { + char* p = static_cast(data); + size_t done = 0; + while (done < len) { + const ssize_t n = read(fd, p + done, len - done); + if (n < 0) { + if (errno == EINTR) { + continue; + } + return false; + } + if (n == 0) { + return false; + } + done += static_cast(n); + } + return true; +} + bool WaitForExit(pid_t pid) { for (int i = 0; i < kPollTimeoutMs / 10; ++i) { int status = 0; @@ -920,7 +1074,315 @@ TEST(ProcfsTaskSemantics, TaskSubtreeSurvivesLeaderExit) { EXPECT_TRUE(WaitForExit(mid)); } + +// --------------------------------------------------------------------------- +// Guardrails for the read paths that changed shape in the same fix +// --------------------------------------------------------------------------- + +// /proc//maps keeps streaming the mappings that exist when the reader asks +// for the next line, the way Linux m_start()/m_next() re-enter the iteration, +// while the fd still holds one line instead of a copy of the whole table: a +// mapping created after the first read shows up in the rest of the stream. +TEST(ProcfsTaskSemantics, MapsStreamsMappingsAddedAfterFirstRead) { + UniqueFd fd(open("/proc/self/maps", O_RDONLY)); + ASSERT_TRUE(fd.valid()) << "cannot open /proc/self/maps: errno=" << errno; + + std::string first; + ASSERT_TRUE(ReadLine(fd.get(), &first)) << "cannot read the first mapping"; + std::string second; + ASSERT_TRUE(ReadLine(fd.get(), &second)) << "cannot read the second mapping"; + + const size_t kLen = 1u << 20; + void* added = mmap(nullptr, kLen, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + ASSERT_NE(MAP_FAILED, added) << "mmap failed: errno=" << errno; + const unsigned long added_start = reinterpret_cast(added); + + std::string rest; + EXPECT_EQ(0, ReadToEof(fd.get(), kReadChunk, &rest)) << "draining /proc/self/maps failed"; + + std::string fresh; + int err = 0; + EXPECT_TRUE(ReadWholePath("/proc/self/maps", &fresh, &err)) << "errno=" << err; + + EXPECT_TRUE(MapsCover(fresh, added_start)) << "/proc/self/maps lost the mapping the test created"; + EXPECT_TRUE(MapsCover(rest, added_start)) + << "a mapping created after the first read never showed up in the stream: the fd " + "served a record that was frozen before it existed"; + + munmap(added, kLen); +} + +// The length of a read must not change the record it returns. /proc//maps +// is rendered one slice at a time, so a single read asking for far more than one +// mapping line reassembles the record from several slices; it must still +// describe exactly what a chunked read of the same position describes. +// +// Both buffers live on the stack, so neither read can move the mapping table the +// other one observes. +TEST(ProcfsTaskSemantics, MapsLargeReadReassemblesTheSameRecord) { + const size_t kMarkerLen = 1u << 20; + void* marker = + mmap(nullptr, kMarkerLen, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + ASSERT_NE(MAP_FAILED, marker) << "mmap failed: errno=" << errno; + const unsigned long marker_start = reinterpret_cast(marker); + + UniqueFd fd(open("/proc/self/maps", O_RDONLY)); + ASSERT_TRUE(fd.valid()) << "cannot open /proc/self/maps: errno=" << errno; + + // Several slices: this is much larger than one mapping line. + char whole[16 * 1024]; + ssize_t n = 0; + do { + n = pread(fd.get(), whole, sizeof(whole), 0); + } while (n < 0 && errno == EINTR); + ASSERT_GT(n, 0) << "large pread failed: errno=" << errno; + + // The same position, read in small chunks. + char chunked[sizeof(whole)]; + ssize_t m = 0; + while (m < static_cast(sizeof(chunked))) { + const ssize_t got = pread(fd.get(), chunked + m, kReadChunk, static_cast(m)); + if (got < 0) { + if (errno == EINTR) { + continue; + } + FAIL() << "chunked pread failed: errno=" << errno; + } + if (got == 0) { + break; + } + m += got; + } + + ASSERT_EQ(n, m) << "the record ends at a different place depending on the read length"; + EXPECT_EQ(0, memcmp(whole, chunked, static_cast(n))) + << "the record a read returns depends on how much it asked for"; + EXPECT_TRUE(MapsCover(std::string(whole, static_cast(n)), marker_start)) + << "the mapping this test created is missing from the record"; + + munmap(marker, kMarkerLen); +} + +// /proc//mountinfo renders the view its open() pinned, the way +// mounts_open_common() takes get_mnt_ns() + get_fs_root(): changing the root +// after opening the fd must not change what that fd reports. +TEST(ProcfsTaskSemantics, MountInfoKeepsTheRootPinnedAtOpen) { + const pid_t child = fork(); + ASSERT_GE(child, 0) << "fork failed: errno=" << errno; + if (child == 0) { + _exit(CheckPinnedMountView()); + } + ReapedChild child_guard(child); + + int status = 0; + bool reaped = false; + for (int i = 0; i < kPollTimeoutMs / 10; ++i) { + if (waitpid(child, &status, WNOHANG) == child) { + reaped = true; + break; + } + usleep(10000); + } + ASSERT_TRUE(reaped) << "the child did not finish"; + child_guard.Disarm(); + ASSERT_TRUE(WIFEXITED(status)) << "the child did not exit normally"; + + const int code = WEXITSTATUS(status); + if (code == 1) { + GTEST_SKIP() << "the guest cannot change its root to show the difference"; + } + EXPECT_EQ(0, code) << "mountinfo followed a root change made after open()"; +} + +// --------------------------------------------------------------------------- +// Guardrails for the streaming read path added on top of the record snapshot +// --------------------------------------------------------------------------- + +// /proc//maps streams one mapping per slice, so killing the target between +// two reads leaves the fd holding the tail of the line it was in the middle of. +// Those bytes are already the reader's: Linux `seq_read_iter()` returns `copied` +// and drops `err` once it copied something, so they must come back instead of +// being thrown away with the `-ESRCH` the *next* record fails with (`m_start()` +// cannot resolve the target any more). +TEST(ProcfsTaskSemantics, MapsStreamKeepsCopiedBytesWhenTargetDies) { + const pid_t child = fork(); + ASSERT_GE(child, 0) << "fork failed: errno=" << errno; + if (child == 0) { + for (;;) { + pause(); + } + } + ReapedChild child_guard(child); + + const std::string path = "/proc/" + std::to_string(child) + "/maps"; + UniqueFd stream(open(path.c_str(), O_RDONLY)); + ASSERT_TRUE(stream.valid()) << "cannot open " << path << ": errno=" << errno; + + // One byte stops the fd inside the first mapping, so the rest of that line + // is still buffered when the target goes away. + char first = 0; + ASSERT_EQ(1, ReadByte(stream.get(), &first)) << "first chunk failed: errno=" << errno; + + std::string whole; + int err = 0; + ASSERT_TRUE(ReadWholePath(path, &whole, &err)) << "cannot read " << path << ": errno=" << err; + ASSERT_GT(whole.size(), 1u) << path << " produced no record"; + ASSERT_EQ(first, whole[0]); + + kill(child, SIGKILL); + ASSERT_TRUE(WaitForExit(child)) << "the target was not reaped"; + child_guard.Disarm(); + + char tail[256]; + ssize_t rest = 0; + do { + rest = read(stream.get(), tail, sizeof(tail)); + } while (rest < 0 && errno == EINTR); + ASSERT_GT(rest, 0) << "the bytes the fd already held were dropped: errno=" << errno; + EXPECT_EQ(whole.substr(1, static_cast(rest)), + std::string(tail, static_cast(rest))) + << "the fd served something other than the record it started"; + + // ... and only then does the unfinished record report its failure. + errno = 0; + EXPECT_EQ(-1, read(stream.get(), tail, sizeof(tail))) + << "the stream continued after the target was gone"; + EXPECT_EQ(ESRCH, errno) << "errno=" << errno; +} + +// The address a /proc//maps fd resumes from only means something inside the +// address space the fd was opened on, so the file pins it the way Linux +// `proc_maps_open()` -> `proc_mem_open()` does. An execve() in the target must +// therefore leave this fd serving the record it started, not continue (or +// restart, or truncate) its stream in the new image. +// +// The helper exec()s this binary again, so the new image has the same mappings +// plus one marker region it reports back; a fresh read of /proc//maps +// proves that image is in place. +TEST(ProcfsTaskSemantics, MapsStreamKeepsTheAddressSpaceOpenedOn) { + int wake_pipe[2] = {-1, -1}; + int report_pipe[2] = {-1, -1}; + ASSERT_EQ(0, pipe(wake_pipe)) << "pipe failed: errno=" << errno; + ASSERT_EQ(0, pipe(report_pipe)) << "pipe failed: errno=" << errno; + + const pid_t child = fork(); + ASSERT_GE(child, 0) << "fork failed: errno=" << errno; + if (child == 0) { + close(wake_pipe[1]); + close(report_pipe[0]); + // The helper must keep both ends across execve(), whatever the default + // close-on-exec state of this kernel is. + fcntl(wake_pipe[0], F_SETFD, 0); + fcntl(report_pipe[1], F_SETFD, 0); + char go = 0; + while (ReadByte(wake_pipe[0], &go) < 0) { + } + char report_fd[16]; + snprintf(report_fd, sizeof(report_fd), "%d", report_pipe[1]); + char* const argv[] = {const_cast("/proc/self/exe"), + const_cast(kMapsExecParkArg), report_fd, nullptr}; + char* const envp[] = {nullptr}; + execve("/proc/self/exe", argv, envp); + _exit(127); + } + + close(wake_pipe[0]); + close(report_pipe[1]); + UniqueFd wake(wake_pipe[1]); + UniqueFd report(report_pipe[0]); + ReapedChild child_guard(child); + + const std::string path = "/proc/" + std::to_string(child) + "/maps"; + UniqueFd stream(open(path.c_str(), O_RDONLY)); + ASSERT_TRUE(stream.valid()) << "cannot open " << path << ": errno=" << errno; + + char first = 0; + ASSERT_EQ(1, ReadByte(stream.get(), &first)) << "first chunk failed: errno=" << errno; + + // The record this fd started on, taken while the target is parked on the + // pipe (so its mappings cannot move under the test). + std::string whole; + int err = 0; + ASSERT_TRUE(ReadWholePath(path, &whole, &err)) << "cannot read " << path << ": errno=" << err; + ASSERT_GT(whole.size(), 1u) << path << " produced no record"; + ASSERT_EQ(first, whole[0]); + + // Let the target execve() and report where its new image mapped a marker. + ASSERT_TRUE(WriteRaw(wake.get(), "x", 1)) << "cannot wake the target: errno=" << errno; + unsigned long marker = 0; + ASSERT_TRUE(ReadRaw(report.get(), &marker, sizeof(marker))) + << "the exec()ed target did not report its marker"; + + std::string fresh; + ASSERT_TRUE(ReadWholePath(path, &fresh, &err)) << "cannot read " << path << ": errno=" << err; + ASSERT_TRUE(MapsCover(fresh, marker)) + << "the exec()ed image never published its marker, so this case cannot observe anything"; + // The two records have to be tellable apart, or serving the new one would + // look the same as serving the old one. Comparing whole records, not the + // marker address on its own: a new mapping can land on an address the old + // table already covered with a different range. + ASSERT_NE(whole, fresh) + << "both records describe the same address space, so this case cannot tell them apart"; + + // The fd keeps serving the address space it was opened on, and only that + // one: it drains the rest of the line it was inside when the target left, + // and the mappings of that address space are gone with the execve() (Linux + // `proc_mem_open()` drops the user reference again, "but do not pin its + // memory"), so `m_start()` reports EOF instead of continuing the stream. + std::string rest; + err = ReadToEof(stream.get(), kReadChunk, &rest); + EXPECT_EQ(0, err) << "continuation read failed: errno=" << err; + std::string reassembled; + reassembled.push_back(first); + reassembled += rest; + const size_t first_line = whole.find('\n'); + ASSERT_NE(std::string::npos, first_line) << "the record has no line break"; + EXPECT_EQ(whole.substr(0, first_line + 1), reassembled) + << "the stream moved on past the address space it was opened on"; +} + +// A position past the end of a record is EOF, not a rewind: reading there +// returns 0 and keeps returning 0, exactly as `seq_lseek()` -> `traverse()` +// leaves the fd, and moving back to 0 renders the record again +// (`seq_read_iter()`'s `ki_pos == 0` reset). +TEST(ProcfsTaskSemantics, SeekPastEndStaysEofAndRewindReRenders) { + UniqueFd fd(open("/proc/version", O_RDONLY)); + ASSERT_TRUE(fd.valid()) << "cannot open /proc/version: errno=" << errno; + + std::string whole; + ASSERT_EQ(0, ReadToEof(fd.get(), kReadChunk, &whole)) << "chunked read failed"; + ASSERT_FALSE(whole.empty()); + + const off_t far = 1 << 20; + ASSERT_EQ(far, lseek(fd.get(), far, SEEK_SET)) << "lseek failed: errno=" << errno; + char buf[16]; + EXPECT_EQ(0, read(fd.get(), buf, sizeof(buf))) << "a seek past the end must report EOF"; + EXPECT_EQ(0, read(fd.get(), buf, sizeof(buf))) << "EOF must stay EOF"; + EXPECT_EQ(0, pread(fd.get(), buf, sizeof(buf), far)) + << "pread() at the same position must report EOF as well"; + + ASSERT_EQ(0, lseek(fd.get(), 0, SEEK_SET)) << "lseek failed: errno=" << errno; + std::string again; + EXPECT_EQ(0, ReadToEof(fd.get(), kReadChunk, &again)) << "rewind failed"; + EXPECT_EQ(whole, again) << "rewinding must render the record again"; +} + int main(int argc, char** argv) { + if (argc >= 3 && strcmp(argv[1], kMapsExecParkArg) == 0) { + const int report_fd = atoi(argv[2]); + const size_t kMarkerLen = 4u << 20; + void* marker = + mmap(nullptr, kMarkerLen, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + const unsigned long addr = + (marker == MAP_FAILED) ? 0UL : reinterpret_cast(marker); + if (addr != 0) { + WriteRaw(report_fd, &addr, sizeof(addr)); + } + close(report_fd); + for (;;) { + pause(); + } + } ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS(); } From dc0164bff53f6d85a216173df350488f13bae628 Mon Sep 17 00:00:00 2001 From: longjin Date: Tue, 15 Sep 2026 18:46:07 +0000 Subject: [PATCH 3/7] fix(procfs): resolve hidden tids to their own task and stream arp per slice Review of #2285 found four places where a procfs node reported another task's state, or buffered more than the file it serves has to: - /proc/net/arp went through proc_read_snapshot(), so every open fd held a copy of the whole neighbour table: N descriptors multiplied an unbounded table by N. It re-sampled the caller's network namespace on every read as well. The file now renders one seq slice at a time through proc_read_seq() and pins the namespace at open(), the way seq_open_net() stores it in the seq private data. - /proc//fd, /proc//fdinfo, the mounts family and /proc//cgroup resolved the thread group leader rather than the task the node names, so a thread that took a private files table (close_range(CLOSE_RANGE_UNSHARE)) or its own fs_struct (unshare(CLONE_FS)) was reported through the leader. They now resolve ProcPidTarget::task(), which is Linux get_proc_task(inode): the leader for /proc/, the thread for /proc/. - /proc//stat took CPU time (fields 14/15) from the per-thread accounting while its fault counters (10/12) aggregated the thread group. Both now come from the one usage view do_task_stat() selects with whole=1, so the two directories of a task group cannot disagree. - The mounts open path reported ESRCH for a task that is gone; mounts_open_common() leaves EINVAL there. The dunitest suite grows to 22 cases: it pins the thread-scoped fd and mounts subtrees and the thread-group stat view against threads holding private state, plus a sliced-read reassembly case for /proc/net/arp. The file now owns a single probe-thread helper instead of two. Signed-off-by: longjin --- kernel/src/filesystem/procfs/mod.rs | 12 +- .../procfs/mount/inode/pid_mount.rs | 20 +- kernel/src/filesystem/procfs/mount/view.rs | 11 +- kernel/src/filesystem/procfs/net/arp.rs | 145 +++- kernel/src/filesystem/procfs/pid/cgroup.rs | 11 +- kernel/src/filesystem/procfs/pid/fd.rs | 16 +- kernel/src/filesystem/procfs/pid/fdinfo.rs | 6 +- kernel/src/filesystem/procfs/pid/mod.rs | 13 +- kernel/src/filesystem/procfs/pid/stat.rs | 27 +- kernel/src/filesystem/procfs/utils.rs | 18 +- kernel/src/net/neighbor/mod.rs | 13 +- .../suites/normal/procfs_task_semantics.cc | 724 +++++++++++++----- 12 files changed, 748 insertions(+), 268 deletions(-) diff --git a/kernel/src/filesystem/procfs/mod.rs b/kernel/src/filesystem/procfs/mod.rs index aba058b4ef..f558091e07 100644 --- a/kernel/src/filesystem/procfs/mod.rs +++ b/kernel/src/filesystem/procfs/mod.rs @@ -8,7 +8,10 @@ use system_error::SystemError; use crate::{ libs::once::Once, - process::{cred::Cred, namespace::pid_namespace::INIT_PID_NAMESPACE, ProcessManager}, + process::{ + cred::Cred, namespace::net_namespace::NetNamespace, + namespace::pid_namespace::INIT_PID_NAMESPACE, ProcessManager, + }, }; use super::vfs::mount::MountFlags; @@ -61,7 +64,7 @@ pub struct ProcfsFilePrivateData { /// still tears the mappings down while this fd stays open. Each read /// re-checks the user count (`mmget_not_zero()`), which is how both files /// learn that there is nothing left to serve. - pub pinned_vm: Option>, + pub(crate) pinned_vm: Option>, /// Streaming state of a seq-style record (Linux `struct seq_file`). Only /// `utils::proc_read_seq()` and `utils::proc_read_snapshot()` touch it. pub(crate) seq: utils::ProcfsSeq, @@ -69,6 +72,10 @@ pub struct ProcfsFilePrivateData { /// `/proc/[pid]/{mounts,mountinfo,mountstats}`, as `mounts_open_common()` /// does; `None` for every other procfs file. pub(crate) mount_view: Option, + /// Network namespace pinned by `open()` for the files under `/proc/net`, + /// as `seq_open_net()` stores it in `seq_net_private`; `None` for every + /// other procfs file. + pub(crate) net_ns: Option>, } impl ProcfsFilePrivateData { @@ -78,6 +85,7 @@ impl ProcfsFilePrivateData { pinned_vm: None, seq: utils::ProcfsSeq::default(), mount_view: None, + net_ns: None, } } } diff --git a/kernel/src/filesystem/procfs/mount/inode/pid_mount.rs b/kernel/src/filesystem/procfs/mount/inode/pid_mount.rs index fb1faa9d46..819ce45d6c 100644 --- a/kernel/src/filesystem/procfs/mount/inode/pid_mount.rs +++ b/kernel/src/filesystem/procfs/mount/inode/pid_mount.rs @@ -43,15 +43,17 @@ impl FileOps for MountProcFileOps { } fn open(&self, data: &mut MutexGuard) -> Result<(), SystemError> { - // Linux `mounts_open_common()` resolves the target once at open time and - // keeps its mount namespace and root path in the seq private data, so a - // `setns()`, `unshare()` or `chroot()` performed afterwards cannot - // change what this fd reports. The record itself is rendered on the - // first read, like any other `seq_file`. - let task = self - .target - .thread_group_leader() - .ok_or(SystemError::ESRCH)?; + // Linux `mounts_open_common()` resolves `get_proc_task(inode)` once at + // open time and keeps its mount namespace and root path in the seq + // private data, so a `setns()`, `unshare()` or `chroot()` performed + // afterwards cannot change what this fd reports. The record itself is + // rendered on the first read, like any other `seq_file`. + // + // The task is the one this node names, not the group leader: a thread + // can unshare its mount namespace or its `fs_struct`, and Linux then + // reports that thread's view. For a `/proc/` node both are the + // leader. + let task = self.target.task().ok_or(SystemError::EINVAL)?; let view = MountView::capture(&task)?; let FilePrivateData::Procfs(pdata) = &mut **data else { return Err(SystemError::EINVAL); diff --git a/kernel/src/filesystem/procfs/mount/view.rs b/kernel/src/filesystem/procfs/mount/view.rs index 8583b12367..9c6724ddb8 100644 --- a/kernel/src/filesystem/procfs/mount/view.rs +++ b/kernel/src/filesystem/procfs/mount/view.rs @@ -38,12 +38,13 @@ impl Debug for MountView { } impl MountView { - /// Pins the view of `task`, whose thread group leader the caller resolved - /// already (`mounts_open_common()` reports `ESRCH` before it gets here). + /// Pins the view of `task`, which the caller resolved from the proc inode + /// already (`mounts_open_common()` reports `EINVAL` for a task that is gone + /// before it gets here). /// - /// The failures follow that function: a task without a root directory is - /// `ENOENT`, like the `!task->fs` check, and a root that is not a mount is - /// `EINVAL`, like its invalid-`root` check. + /// A task without a root directory is `ENOENT`, like the `!task->fs` check + /// there. A root that is not a mount cannot happen for one that has an + /// `fs_struct`, so that arm is defensive. pub(crate) fn capture(task: &Arc) -> Result { let ns = task.nsproxy().mnt_ns.clone(); let root = task diff --git a/kernel/src/filesystem/procfs/net/arp.rs b/kernel/src/filesystem/procfs/net/arp.rs index b2aadb6b29..ad36d782fb 100644 --- a/kernel/src/filesystem/procfs/net/arp.rs +++ b/kernel/src/filesystem/procfs/net/arp.rs @@ -8,16 +8,22 @@ use crate::filesystem::{ procfs::{ template::{Builder, FileOps, ProcFileBuilder}, - utils::proc_read_snapshot, + utils::proc_read_seq, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }; use crate::libs::mutex::MutexGuard; -use crate::net::neighbor; +use crate::net::neighbor::{self, ArpEntry}; +use crate::process::namespace::net_namespace::NetNamespace; +use crate::process::ProcessManager; use alloc::string::ToString; use alloc::{string::String, sync::Arc, sync::Weak, vec::Vec}; use system_error::SystemError; +/// Header row, byte for byte the `seq_puts()` of Linux `arp_seq_show()`. +const ARP_HEADER: &[u8] = + b"IP address HW type Flags HW address Mask Device\n"; + /// /proc/net/arp 文件的 FileOps 实现 #[derive(Debug)] pub struct ArpFileOps; @@ -29,53 +35,101 @@ impl ArpFileOps { .build() .unwrap() } +} - fn generate_arp_content() -> Vec { - let mut content = String::from( - "IP address HW type Flags HW address Mask Device\n", - ); +/// Formats one row the way Linux `arp_format_neigh_entry()` does. +fn format_arp_entry(entry: &ArpEntry) -> String { + // Linux uses %-16s for IPv4 strings (see net/ipv4/arp.c: arp_format_neigh_entry) + // smoltcp::wire::IpAddress's Display implementation does not honor formatter width, + // so we stringify first and apply padding to the String. + let ip_str = entry.ip_addr.to_string(); - // 调用网络子系统的API获取ARP条目 - let entries = neighbor::get_arp_entries(); + // Linux prints MAC as lowercase hex with ':' separators. + let hw_addr_str = match entry.hw_addr { + smoltcp::wire::HardwareAddress::Ethernet(eth) => { + let b = eth.0; + format!( + "{:02x}:{:02x}:{:02x}:{:02x}:{:02x}:{:02x}", + b[0], b[1], b[2], b[3], b[4], b[5] + ) + } + _ => entry + .hw_addr + .to_string() + .replace('-', ":") + .to_ascii_lowercase(), + }; - // 格式化输出每个条目 - for entry in entries { - // Linux uses %-16s for IPv4 strings (see net/ipv4/arp.c: arp_format_neigh_entry) - // smoltcp::wire::IpAddress's Display implementation does not honor formatter width, - // so we stringify first and apply padding to the String. - let ip_str = entry.ip_addr.to_string(); + format!( + "{:<16} 0x{:<10x}0x{:<10x}{:<17} * {}\n", + ip_str, + entry.hw_type.as_u16(), + entry.flags.bits(), + hw_addr_str, + entry.device, + ) +} - // Linux prints MAC as lowercase hex with ':' separators. - let hw_addr_str = match entry.hw_addr { - smoltcp::wire::HardwareAddress::Ethernet(eth) => { - let b = eth.0; - format!( - "{:02x}:{:02x}:{:02x}:{:02x}:{:02x}:{:02x}", - b[0], b[1], b[2], b[3], b[4], b[5] - ) - } - _ => entry - .hw_addr - .to_string() - .replace('-', ":") - .to_ascii_lowercase(), - }; +/// Renders the ARP table from `cursor` on, the way Linux `arp_seq_ops` +/// (`net/ipv4/arp.c`) walks it through `neigh_seq_start()`/`neigh_seq_next()`, +/// and stops once `want` bytes are out. +/// +/// `cursor` is the index of the next entry to render, which is the position +/// `neigh_seq_start()` keeps in `*pos`. It is `None` for the first slice of a +/// record, which also emits the header. Returns the index the next slice +/// resumes at, or `None` when the table ends here. +/// +/// One slice per call is what keeps an fd from holding a copy of the whole +/// table, the way `seq_file` holds one block: the neighbour table has no +/// capacity limit, so a process that accumulates neighbours must not be able to +/// multiply it by its descriptor count. +/// +/// `netns` is the namespace `open()` pinned, not the reader's current one: the +/// walk has to keep reading the table it started on, the way `seq_open_net()` +/// keeps `seq_net_private.net` for the whole life of the fd. +fn render_arp_slice( + netns: &Arc, + cursor: Option, + want: usize, + out: &mut Vec, +) -> Option { + let entries = neighbor::get_arp_entries(netns); + let mut index = cursor.unwrap_or(0); + if cursor.is_none() { + out.extend_from_slice(ARP_HEADER); + } - content.push_str(&format!( - "{:<16} 0x{:<10x}0x{:<10x}{:<17} * {}\n", - ip_str, - entry.hw_type.as_u16(), - entry.flags.bits(), - hw_addr_str, - entry.device, - )); + while index < entries.len() { + let line = format_arp_entry(&entries[index]); + // The first row of a slice is always admitted, the way `seq_file` grows + // its buffer for a record that does not fit: a reader that asked for + // fewer bytes than one row still advances. + if !out.is_empty() && out.len() + line.len() > want { + break; } + out.extend_from_slice(line.as_bytes()); + index += 1; + } - content.into_bytes() + if index < entries.len() { + Some(index) + } else { + None } } impl FileOps for ArpFileOps { + fn open(&self, data: &mut MutexGuard) -> Result<(), SystemError> { + // Linux `seq_open_net()` pins `get_proc_net(inode)` in the seq private + // data when the file is opened, so a `setns()` afterwards cannot make + // one fd report two tables. + let FilePrivateData::Procfs(pdata) = &mut **data else { + return Err(SystemError::EINVAL); + }; + pdata.net_ns = Some(ProcessManager::current_netns()); + Ok(()) + } + fn read_at( &self, offset: usize, @@ -83,10 +137,17 @@ impl FileOps for ArpFileOps { buf: &mut [u8], mut data: MutexGuard, ) -> Result { - // `seq_file` (Linux `proc_create_net(&arp_seq_ops)`, `net/ipv4/arp.c`): one - // fd sees one rendered record. - proc_read_snapshot(offset, len, buf, &mut data, || { - Ok(Self::generate_arp_content()) + // `seq_file` (`proc_create_net("arp", 0444, net->proc_net, + // &arp_seq_ops, ...)`, `net/ipv4/arp.c`): one fd holds the block it has + // not drained yet, never the whole table. + let netns = { + let FilePrivateData::Procfs(pdata) = &*data else { + return Err(SystemError::EINVAL); + }; + pdata.net_ns.clone().ok_or(SystemError::EINVAL)? + }; + proc_read_seq(offset, len, buf, &mut data, |cursor, want, out| { + Ok(render_arp_slice(&netns, cursor, want, out)) }) } } diff --git a/kernel/src/filesystem/procfs/pid/cgroup.rs b/kernel/src/filesystem/procfs/pid/cgroup.rs index 00428f9bda..272627d639 100644 --- a/kernel/src/filesystem/procfs/pid/cgroup.rs +++ b/kernel/src/filesystem/procfs/pid/cgroup.rs @@ -34,10 +34,13 @@ impl CgroupFileOps { } fn generate_content(&self) -> Result, SystemError> { - let target = self - .target - .thread_group_leader() - .ok_or(SystemError::ESRCH)?; + // Linux `proc_cgroup_show()` runs on `get_proc_task(inode)`, so a + // hidden tid reports the membership of the thread it names. The only + // migration path, `cgroup.procs`, moves the whole thread group and + // skips its exited members, exactly as `cgroup_attach_task()` does with + // `while_each_thread()` over the `PF_EXITING` check, so the two + // directories still agree. + let target = self.target.task().ok_or(SystemError::ESRCH)?; let viewer = ProcessManager::current_pcb(); let target_cg = target.task_cgroup_node(); diff --git a/kernel/src/filesystem/procfs/pid/fd.rs b/kernel/src/filesystem/procfs/pid/fd.rs index 08ae7d82bb..de3fc9e181 100644 --- a/kernel/src/filesystem/procfs/pid/fd.rs +++ b/kernel/src/filesystem/procfs/pid/fd.rs @@ -34,7 +34,12 @@ impl FdDirOps { } fn get_process(&self) -> Option> { - self.target.thread_group_leader() + // Linux resolves the directory's `get_proc_task(inode)`, so a hidden + // tid shows the files table of *that thread*: a thread that took a + // private one with `close_range(CLOSE_RANGE_UNSHARE)` must not read the + // leader's. For `/proc/` the two agree, because the tgid resolves + // to the leader. + self.target.task() } } @@ -140,10 +145,9 @@ impl FdSymOps { impl SymOps for FdSymOps { fn read_link(&self, buf: &mut [u8]) -> Result { - let process = self - .target - .thread_group_leader() - .ok_or(SystemError::ENOENT)?; + // `proc_fd_link()` walks `get_proc_task(inode)->files`, so the link is + // resolved against the thread this node names, not the group leader. + let process = self.target.task().ok_or(SystemError::ENOENT)?; // 先获取文件对象的 clone，然后立即释放 fd_table 锁 // 避免在持有锁时调用可能获取其他锁的方法（如 absolute_path） @@ -208,7 +212,7 @@ impl SymOps for FdSymOps { } fn special_node(&self) -> Option { - let process = self.target.thread_group_leader()?; + let process = self.target.task()?; // 获取文件对象 let file = { diff --git a/kernel/src/filesystem/procfs/pid/fdinfo.rs b/kernel/src/filesystem/procfs/pid/fdinfo.rs index 7a1db22da2..525fdfc9b9 100644 --- a/kernel/src/filesystem/procfs/pid/fdinfo.rs +++ b/kernel/src/filesystem/procfs/pid/fdinfo.rs @@ -33,7 +33,9 @@ impl FdInfoDirOps { } fn get_process(&self) -> Option> { - self.target.thread_group_leader() + // `proc_readfd_common()`/`seq_show()` walk `get_proc_task(inode)`, so a + // hidden tid reports the descriptor table of the thread it names. + self.target.task() } } @@ -132,7 +134,7 @@ impl FdInfoFileOps { } fn is_current(&self) -> bool { - let Some(process) = self.target.thread_group_leader() else { + let Some(process) = self.target.task() else { return false; }; let Some(fd_table) = process.basic().try_fd_table().clone() else { diff --git a/kernel/src/filesystem/procfs/pid/mod.rs b/kernel/src/filesystem/procfs/pid/mod.rs index 788f7dc741..da6dcc353c 100644 --- a/kernel/src/filesystem/procfs/pid/mod.rs +++ b/kernel/src/filesystem/procfs/pid/mod.rs @@ -115,6 +115,14 @@ impl ProcPidTarget { self.pid.pid_nr_ns(&self.view_pid_ns) } + /// The task this node names, the way Linux `get_proc_task(inode)` resolves + /// the proc inode. A `/proc/` node names the group leader, a + /// `/proc/` node the thread, and either can be gone. + /// + /// Files that report per-task state (`files`, `fs_struct`, `nsproxy`) read + /// from here; files backed by state the whole group shares (`mm`, + /// `sighand`, credentials) may use [`Self::thread_group_leader()`] instead, + /// which is the same task for a `/proc/` node. pub fn task(&self) -> Option> { self.pid.pid_task(PidType::PID) } @@ -170,8 +178,11 @@ impl PidDirOps { .unwrap() } + /// The task this directory names, for the entries that only exist when it + /// does (`fd`/`fdinfo`, which Linux creates with `proc_pid_make_inode()` on + /// the same `get_proc_task(inode)` their readers use). fn get_process(&self) -> Option> { - self.target.thread_group_leader() + self.target.task() } pub(super) fn is_current_target(&self) -> bool { diff --git a/kernel/src/filesystem/procfs/pid/stat.rs b/kernel/src/filesystem/procfs/pid/stat.rs index c383d60eb6..6c69016da7 100644 --- a/kernel/src/filesystem/procfs/pid/stat.rs +++ b/kernel/src/filesystem/procfs/pid/stat.rs @@ -3,7 +3,6 @@ //! 以单行格式返回进程的状态信息，兼容 Linux procfs 格式 use core::fmt::Write; -use core::sync::atomic::Ordering; use crate::libs::mutex::MutexGuard; use crate::{ @@ -178,9 +177,10 @@ fn generate_linux_proc_stat_line(snapshot: &ProcStatSnapshot) -> String { line.push_ull(snapshot.majflt as u64); // 12 majflt line.push_ull(snapshot.cmajflt as u64); // 13 cmajflt - // 14/15: CPU time of this task. For `whole=1` (i.e. `/proc//stat`) - // Linux aggregates the whole thread group; DragonOS does not implement that - // aggregation yet, which is a pre-existing deviation. + // 14/15: CPU time under the accounting view the caller selected, the same + // one behind fields 10/12. Linux `whole=1` (i.e. `/proc//stat` and a + // hidden-tid `/proc//stat`) aggregates the whole thread group, while + // `/proc//task//stat` reports the thread alone. line.push_ull(snapshot.utime); // 14 utime line.push_ull(snapshot.stime); // 15 stime line.push_ll(0); // 16 cutime @@ -255,14 +255,21 @@ impl StatFileOps { .map(|tty| tty.core().device_number().new_encode_dev() as i32) .unwrap_or(0) }; - let cpu_time = pcb.cputime(); - let utime = ns_to_clock_t(cpu_time.utime.load(Ordering::Relaxed)); - let stime = ns_to_clock_t(cpu_time.stime.load(Ordering::Relaxed)); - let fault_usage = match self.scope { + let usage = match self.scope { StatScope::ThreadGroup => pcb.get_rusage(RUsageWho::RUsageSelf), StatScope::Thread => pcb.get_rusage(RUsageWho::RusageThread), } .unwrap_or_default(); + // Fields 14/15 come from the same accounting view as the fault counters + // in 10/12: Linux `do_task_stat()` takes `thread_group_cputime_adjusted()` + // when `whole` is set, and `/proc//stat` is `proc_tgid_stat()` + // (whole = 1) even when `nr` names a non-leader thread. Reporting the + // faults of the group next to the CPU time of one thread would make two + // directories of the same task group disagree about the group. + let (utime, stime) = ( + ns_to_clock_t(usage.ru_utime.to_ns()), + ns_to_clock_t(usage.ru_stime.to_ns()), + ); let child_usage = pcb .get_rusage(RUsageWho::RUsageChildren) .unwrap_or_default(); @@ -320,9 +327,9 @@ impl StatFileOps { policy, utime, stime, - minflt: fault_usage.ru_minflt, + minflt: usage.ru_minflt, cminflt: child_usage.ru_minflt, - majflt: fault_usage.ru_majflt, + majflt: usage.ru_majflt, cmajflt: child_usage.ru_majflt, }); Ok(content.into_bytes()) diff --git a/kernel/src/filesystem/procfs/utils.rs b/kernel/src/filesystem/procfs/utils.rs index 56240cc3e7..fdc25ae9a1 100644 --- a/kernel/src/filesystem/procfs/utils.rs +++ b/kernel/src/filesystem/procfs/utils.rs @@ -95,6 +95,10 @@ impl ProcfsSeq { /// how much a seq file buffers. Bounding a slice the same way keeps a large read /// from turning into a large per-fd buffer for a record source that can render /// arbitrarily much, such as the mapping table of `/proc/[pid]/maps`. +/// +/// Only an incremental source is held to it: the sources that render one whole +/// record ([`proc_read_snapshot()`], i.e. Linux `single_open()`) put that record +/// in the fd's buffer in one piece, which is what makes an fd a snapshot of it. const SEQ_SLICE_MAX: usize = MMArch::PAGE_SIZE; /// Serves one procfs record the way Linux `seq_read_iter()` does. @@ -106,10 +110,12 @@ const SEQ_SLICE_MAX: usize = MMArch::PAGE_SIZE; /// /// `source` is handed the cursor of the previous slice (`None` for the first /// slice of a record), how many bytes the reader still wants, and an empty -/// buffer to render into. The wanted byte count is an upper bound on the slice, -/// never more than [`SEQ_SLICE_MAX`], so an incremental source may have to be -/// entered several times before one read is satisfied. It returns the cursor to -/// resume from, or `None` when the record ends after this slice. +/// buffer to render into. The wanted byte count is a budget, never more than +/// [`SEQ_SLICE_MAX`]: an incremental source stops after roughly that much and is +/// entered again for the next slice, so one read may need several slices, while +/// a source that renders one whole record ignores the budget on purpose. The +/// source returns the cursor to resume from, or `None` when the record ends +/// after this slice. /// /// Position rules mirror `seq_read_iter()`/`seq_lseek()`: /// - `offset == 0`: rewind, so the record is rendered again; @@ -120,8 +126,8 @@ const SEQ_SLICE_MAX: usize = MMArch::PAGE_SIZE; /// - a seek past the end of the record parks the fd at the requested offset, so /// reading there reports EOF instead of rendering the record again. /// -/// An empty request (`len == 0`, or a full buffer) returns 0 without touching -/// the fd, and a source that fails after this call already copied bytes out +/// An empty request (`len == 0`, or a full buffer) returns 0 without rendering +/// anything, and a source that fails after this call already copied bytes out /// still reports those bytes, as `seq_read_iter()` returns `copied` and discards /// the error once it copied something. pub(super) fn proc_read_seq( diff --git a/kernel/src/net/neighbor/mod.rs b/kernel/src/net/neighbor/mod.rs index 9a30c04f75..9436f35f22 100644 --- a/kernel/src/net/neighbor/mod.rs +++ b/kernel/src/net/neighbor/mod.rs @@ -18,7 +18,7 @@ use crate::{ routing::uapi::arp::{ArpFlags, ArpHrd}, rtnl::RtnlGuard, }, - process::{namespace::net_namespace::NetNamespace, ProcessManager}, + process::namespace::net_namespace::NetNamespace, }; pub(crate) use table::{NeighborReadGuard, NeighborSnapshot, NeighborTable}; @@ -207,12 +207,15 @@ pub(crate) fn release_deferred_after_enqueue( common.release_configured_neighbor(ifindex, next_hop) } -/// Returns the current netns ARP view. Configured permanent entries shadow a +/// Returns the ARP view of `netns`. Configured permanent entries shadow a /// dynamic smoltcp entry with the same `(ifindex, IPv4)` key. Linux omits /// NUD_NOARP entries from `/proc/net/arp`. -pub fn get_arp_entries() -> Vec { - let netns = ProcessManager::current_netns(); - let configured = match snapshot(&netns) { +/// +/// The caller names the namespace instead of the function taking the current +/// one, because `/proc/net/arp` pins it at open (`seq_open_net()`) and a read +/// must not mix two namespaces into one record. +pub fn get_arp_entries(netns: &Arc) -> Vec { + let configured = match snapshot(netns) { Ok(entries) => entries, Err(error) => { log::warn!("failed to snapshot configured ARP entries: {:?}", error); diff --git a/user/apps/tests/dunitest/suites/normal/procfs_task_semantics.cc b/user/apps/tests/dunitest/suites/normal/procfs_task_semantics.cc index bd3eb5f294..d16a6f7378 100644 --- a/user/apps/tests/dunitest/suites/normal/procfs_task_semantics.cc +++ b/user/apps/tests/dunitest/suites/normal/procfs_task_semantics.cc @@ -1,6 +1,7 @@ // procfs task semantics (issue #2283). // -// Three behaviours are pinned here, each against the Linux 6.6 model: +// The behaviours pinned here follow the Linux 6.6 model, one section per +// thread of the analysis: // // 1. one fd sees one record. Linux serves these files through // single_open()/seq_read_iter(), so a read() that reached EOF keeps @@ -12,7 +13,10 @@ // so /proc//task//status and /proc//status agree on Ppid; // 3. /proc/ resolves any task that still holds a PID link (Linux // proc_pid_lookup() -> find_task_by_pid_ns()), while /proc *lists* group -// leaders only (Linux next_tgid()). +// leaders only (Linux next_tgid()). Everything below such a directory +// reads the task it names: the fd and fdinfo subtrees walk its files +// table, mounts/mountinfo render its root, and stat reports whole +// thread-group accounting even for a hidden tid. // // Companion analysis: // docs/kernel/filesystem/proc/procfs-task-semantics-root-cause.md @@ -34,6 +38,7 @@ #include #include #include +#include #include #include @@ -272,84 +277,6 @@ class CommGuard { std::string saved_; }; -// Long-lived worker thread: publishes its tid, then blocks until the owner -// closes the release pipe. The destructor always joins, so a failing ASSERT -// cannot leave a blocked thread behind for the next case. -class Worker { -public: - Worker() = default; - ~Worker() { Stop(); } - Worker(const Worker&) = delete; - Worker& operator=(const Worker&) = delete; - - bool Start() { - if (pipe(ready_) != 0 || pipe(release_) != 0) { - return false; - } - if (pthread_create(&thread_, nullptr, Main, this) != 0) { - return false; - } - started_ = true; - char byte = 0; - ssize_t n = 0; - do { - n = read(ready_[0], &byte, 1); - } while (n < 0 && errno == EINTR); - if (n != 1) { - Stop(); - return false; - } - return true; - } - - void Stop() { - if (release_[0] >= 0) { - close(release_[0]); - release_[0] = -1; - } - if (release_[1] >= 0) { - close(release_[1]); - release_[1] = -1; - } - if (started_) { - pthread_join(thread_, nullptr); - started_ = false; - } - if (ready_[0] >= 0) { - close(ready_[0]); - ready_[0] = -1; - } - if (ready_[1] >= 0) { - close(ready_[1]); - ready_[1] = -1; - } - } - - long tid() const { return tid_; } - -private: - static void* Main(void* arg) { - Worker* self = static_cast(arg); - prctl(PR_SET_NAME, "worker", 0, 0, 0); - self->tid_ = GetTid(); - const char ready = 'r'; - if (write(self->ready_[1], &ready, 1) != 1) { - return nullptr; - } - char buf[8]; - while (read(self->release_[0], buf, sizeof(buf)) > 0) { - } - return nullptr; - } - - int ready_[2] = {-1, -1}; - int release_[2] = {-1, -1}; - long tid_ = 0; - pthread_t thread_ = {}; - bool started_ = false; -}; - - // Reads one line with 1-byte reads, leaving the fd on a line boundary. bool ReadLine(int fd, std::string* line) { line->clear(); @@ -494,6 +421,133 @@ bool ReadRaw(int fd, void* data, size_t len) { return true; } +/// Channels a probe thread exchanges with its owner: one fixed-size report out, +/// a park until the release pipe reports EOF, and the owner's own argument, the +/// way pthread_create() hands one to a start routine. +struct ProbePayload { + int report_wfd; + int release_rfd; + void* arg; +}; + +/// Blocks until the owner closes the release pipe. +void ParkUntilReleased(int release_rfd) { + char buf[8]; + while (read(release_rfd, buf, sizeof(buf)) > 0) { + } +} + +/// Owns a probe thread, the pipes it reports over, and a join that always runs, +/// so a failed assertion cannot leave the thread parked behind the case. +/// +/// Prepare() opens the pipes before Launch() starts the body, so a body that +/// needs to name a pipe descriptor can be handed it in its argument. The table +/// is shared with the thread, so a body that gave itself a private copy has to +/// close its own copy of the release write end, or the park below can never see +/// the owner release it. +class ProbeThread { +public: + using Body = void* (*)(void*); + + ProbeThread() = default; + ~ProbeThread() { Stop(); } + ProbeThread(const ProbeThread&) = delete; + ProbeThread& operator=(const ProbeThread&) = delete; + + bool Prepare() { + if (pipe(report_) != 0 || pipe(release_) != 0) { + return false; + } + payload_.report_wfd = report_[1]; + payload_.release_rfd = release_[0]; + return true; + } + + /// The descriptor a body has to leave alone, or close in a private copy of + /// the table, to make the parked thread release. + int release_write_fd() const { return release_[1]; } + + bool Launch(Body body, void* arg) { + payload_.arg = arg; + if (pthread_create(&thread_, nullptr, body, &payload_) != 0) { + return false; + } + started_ = true; + return true; + } + + /// The thread's fixed-size report. The pipes stay open until Stop(), the + /// way the descriptor table is shared with the thread. + bool ReadReport(void* out, size_t len) { return ReadRaw(report_[0], out, len); } + + void Stop() { + if (release_[1] >= 0) { + close(release_[1]); + release_[1] = -1; + } + if (started_) { + pthread_join(thread_, nullptr); + started_ = false; + } + for (int i = 0; i < 2; ++i) { + if (report_[i] >= 0) { + close(report_[i]); + report_[i] = -1; + } + if (release_[i] >= 0) { + close(release_[i]); + release_[i] = -1; + } + } + } + +private: + ProbePayload payload_ = {-1, -1, nullptr}; + int report_[2] = {-1, -1}; + int release_[2] = {-1, -1}; + pthread_t thread_ = {}; + bool started_ = false; +}; + +/// Names itself "worker", publishes its tid, then parks: the plain probe thread +/// the tid-addressed cases send in. +void* TidWorker(void* arg) { + ProbePayload* payload = static_cast(arg); + prctl(PR_SET_NAME, "worker", 0, 0, 0); + const long tid = GetTid(); + WriteRaw(payload->report_wfd, &tid, sizeof(tid)); + ParkUntilReleased(payload->release_rfd); + return nullptr; +} + +/// Long-lived worker thread: publishes its tid, then blocks until the owner +/// closes the release pipe. The destructor always joins, so a failing ASSERT +/// cannot leave a blocked thread behind for the next case. +class Worker { +public: + Worker() = default; + ~Worker() { Stop(); } + Worker(const Worker&) = delete; + Worker& operator=(const Worker&) = delete; + + bool Start() { + if (!probe_.Prepare() || !probe_.Launch(TidWorker, nullptr) || + !probe_.ReadReport(&tid_, sizeof(tid_))) { + Stop(); + return false; + } + return true; + } + + void Stop() { probe_.Stop(); } + + long tid() const { return tid_; } + +private: + ProbeThread probe_; + long tid_ = 0; +}; + bool WaitForExit(pid_t pid) { for (int i = 0; i < kPollTimeoutMs / 10; ++i) { int status = 0; @@ -802,7 +856,7 @@ TEST(ProcfsTaskSemantics, OomScoreAdjStaysStream) { // 2. thread-level re-parenting // --------------------------------------------------------------------------- -struct ReparentReport { +struct GroupReparentReport { long original_parent; long new_parent; long leader_ppid; @@ -810,69 +864,49 @@ struct ReparentReport { long worker_tid; }; -// Publishes the worker's tid over a pipe and then stays alive long enough for -// the owner to observe the thread group. -void* PublishTidWorker(void* arg) { - const int wfd = *static_cast(arg); - prctl(PR_SET_NAME, "worker", 0, 0, 0); - const long tid = GetTid(); - const ssize_t ignored = write(wfd, &tid, sizeof(tid)); - (void)ignored; - for (int i = 0; i < 400; ++i) { - usleep(25000); - } - return nullptr; -} - -// Read a `long` published by PublishTidWorker; -1 when the pipe closed short. -long ReadPublishedTid(int rfd) { - long tid = 0; - size_t got = 0; - while (got < sizeof(tid)) { - const ssize_t n = read(rfd, reinterpret_cast(&tid) + got, sizeof(tid) - got); - if (n <= 0) { - break; - } - got += static_cast(n); - } - return got == sizeof(tid) ? tid : -1; -} - // When a thread group is re-parented, every thread must report the new parent: // /proc//task//status used to keep the dead parent while // /proc//status reported the adopter. +// +// The parent is not put to sleep and hoped to still be there: it waits on a +// pipe until the group has read the Ppid that names it and has its second +// thread parked, so the observation cannot lose the race against the exit that +// triggers the re-parenting. TEST(ProcfsTaskSemantics, ThreadPpidFollowsGroupReparent) { - int pipefd[2]; - ASSERT_EQ(0, pipe(pipefd)) << "pipe failed: errno=" << errno; - - const pid_t top = fork(); - ASSERT_GE(top, 0) << "fork failed: errno=" << errno; - if (top == 0) { - close(pipefd[0]); - const pid_t mid = fork(); - if (mid == 0) { - int tidpipe[2]; - if (pipe(tidpipe) != 0) { - _exit(3); - } - pthread_t th; - if (pthread_create(&th, nullptr, PublishTidWorker, &tidpipe[1]) != 0) { + int ready[2]; + int report[2]; + ASSERT_EQ(0, pipe(ready)) << "pipe failed: errno=" << errno; + ASSERT_EQ(0, pipe(report)) << "pipe failed: errno=" << errno; + + const pid_t dying = fork(); + ASSERT_GE(dying, 0) << "fork failed: errno=" << errno; + if (dying == 0) { + // This process only exists to die: its exit is what re-parents the + // group below. The pipes are duplicated into that group, so the + // descriptors this process does not need are closed only after the + // fork that created it. + close(report[0]); + const pid_t group = fork(); + if (group == 0) { + close(ready[0]); + GroupReparentReport rep = {}; + Worker worker; + if (!worker.Start()) { _exit(3); } - const long worker_tid = ReadPublishedTid(tidpipe[0]); - close(tidpipe[0]); - close(tidpipe[1]); - if (worker_tid <= 0) { + rep.worker_tid = worker.tid(); + rep.original_parent = getppid(); + + // Let the parent go only now that `original_parent` is sampled, and + // keep the second thread parked until both views were read back, so + // neither number can be lost to a dead target. + const char go = 'g'; + if (write(ready[1], &go, 1) != 1) { _exit(3); } + close(ready[1]); - ReparentReport rep = {}; - rep.worker_tid = worker_tid; - rep.original_parent = getppid(); - for (int i = 0; i < 2000; ++i) { - if (getppid() != rep.original_parent) { - break; - } + for (int i = 0; i < 2000 && getppid() == rep.original_parent; ++i) { usleep(5000); } rep.new_parent = getppid(); @@ -884,38 +918,38 @@ TEST(ProcfsTaskSemantics, ThreadPpidFollowsGroupReparent) { } else { rep.leader_ppid = -1; } - const std::string thread_path = TaskStatusPath(getpid(), rep.worker_tid); read_err = 0; - if (ReadWholePath(thread_path, &text, &read_err)) { + if (ReadWholePath(TaskStatusPath(getpid(), rep.worker_tid), &text, &read_err)) { rep.thread_ppid = strtol(Field(ParseStatus(text), "Ppid").c_str(), nullptr, 10); } else { rep.thread_ppid = -read_err; } - const ssize_t ignored = write(pipefd[1], &rep, sizeof(rep)); - (void)ignored; - _exit(0); + + const bool reported = WriteRaw(report[1], &rep, sizeof(rep)); + worker.Stop(); + _exit(reported ? 0 : 3); } - // The middle process is the one that dies; its exit is what re-parents - // the worker's whole thread group. - usleep(400000); + close(ready[1]); + close(report[1]); + char token = 0; + ssize_t n = 0; + do { + n = read(ready[0], &token, 1); + } while (n < 0 && errno == EINTR); + close(ready[0]); _exit(0); } - close(pipefd[1]); - ReparentReport rep = {}; - size_t got = 0; - while (got < sizeof(rep)) { - const ssize_t n = read(pipefd[0], reinterpret_cast(&rep) + got, sizeof(rep) - got); - if (n <= 0) { - break; - } - got += static_cast(n); - } - close(pipefd[0]); + close(ready[0]); + close(ready[1]); + close(report[1]); + GroupReparentReport rep = {}; + const bool got = ReadRaw(report[0], &rep, sizeof(rep)); + close(report[0]); int status = 0; - waitpid(top, &status, 0); + waitpid(dying, &status, 0); - ASSERT_EQ(sizeof(rep), got) << "the re-parented thread group did not report"; + ASSERT_TRUE(got) << "the re-parented thread group did not report"; ASSERT_GT(rep.original_parent, 0L); ASSERT_GT(rep.new_parent, 0L); EXPECT_NE(rep.original_parent, rep.new_parent) @@ -1019,20 +1053,16 @@ TEST(ProcfsTaskSemantics, TaskSubtreeSurvivesLeaderExit) { ASSERT_GE(mid, 0) << "fork failed: errno=" << errno; if (mid == 0) { close(pipefd[0]); - int tidpipe[2]; - if (pipe(tidpipe) != 0) { + Worker worker; + if (!worker.Start()) { _exit(3); } - pthread_t th; - if (pthread_create(&th, nullptr, PublishTidWorker, &tidpipe[1]) != 0) { + const long tid = worker.tid(); + const bool reported = WriteRaw(pipefd[1], &tid, sizeof(tid)); + close(pipefd[1]); + if (!reported) { _exit(3); } - const long tid = ReadPublishedTid(tidpipe[0]); - close(tidpipe[0]); - close(tidpipe[1]); - const ssize_t ignored = write(pipefd[1], &tid, sizeof(tid)); - (void)ignored; - close(pipefd[1]); usleep(100000); // Only the group leader must exit, leaving the group alive. This uses // exit(2) rather than pthread_exit(): the latter performs a forced @@ -1043,18 +1073,25 @@ TEST(ProcfsTaskSemantics, TaskSubtreeSurvivesLeaderExit) { close(pipefd[1]); long worker_tid = 0; - size_t got = 0; - while (got < sizeof(worker_tid)) { - const ssize_t n = read(pipefd[0], reinterpret_cast(&worker_tid) + got, - sizeof(worker_tid) - got); - if (n <= 0) { - break; + const bool published = ReadRaw(pipefd[0], &worker_tid, sizeof(worker_tid)); + close(pipefd[0]); + ASSERT_TRUE(published) << "the worker tid was not published"; + + // Wait for the leader to actually be gone instead of assuming it after a + // fixed delay: every assertion below is only about the shape the group has + // once its leader exited. + bool leader_exited = false; + for (int i = 0; i < kPollTimeoutMs / 10 && !leader_exited; ++i) { + std::string text; + int err = 0; + if (ReadWholePath(StatusPath(mid), &text, &err)) { + leader_exited = Field(ParseStatus(text), "State").find("Exited") != std::string::npos; + } + if (!leader_exited) { + usleep(10000); } - got += static_cast(n); } - close(pipefd[0]); - ASSERT_EQ(sizeof(worker_tid), got) << "the worker tid was not published"; - usleep(300000); + ASSERT_TRUE(leader_exited) << "the group leader never exited"; const std::vector tids = ListDir(StatusPath(mid, "task")); EXPECT_EQ(2u, tids.size()) << "the task subtree must list the zombie leader and the worker"; @@ -1367,6 +1404,341 @@ TEST(ProcfsTaskSemantics, SeekPastEndStaysEofAndRewindReRenders) { EXPECT_EQ(whole, again) << "rewinding must render the record again"; } +// --------------------------------------------------------------------------- +// 4. per-thread state behind a hidden tid +// --------------------------------------------------------------------------- +// +// Linux builds /proc/ from the task proc_pid_lookup() found, so everything +// below it reads *that* task: proc_fd_link()/proc_readfd_common() walk its +// files table, mounts_open_common() pins its mount namespace and root, and +// proc_tgid_stat() still reports whole-thread-group accounting. The cases below +// pin that against a thread which took private state, and against a hidden tid +// whose stat record used to mix the per-thread and thread-group views. + +#ifndef SYS_close_range +#define SYS_close_range 436 +#endif +#ifndef SYS_unshare +#define SYS_unshare 272 +#endif + +/// close_range(2) flag that gives the calling thread its own files table. +constexpr unsigned kCloseRangeUnshare = 1u << 1; +/// unshare(2) flag that gives the calling thread its own fs_struct. +constexpr unsigned kCloneFs = 0x00000200; +namespace { + +/// Reports a thread's tid plus what its private-table setup did. +struct PrivateTableReport { + long tid; + int unshare_errno; +}; + +/// What the private-table probe has to name: the descriptor it punches out of +/// its own copy of the table, and the release write end it has to close in that +/// copy before parking. +struct PrivateTableArg { + int punch_fd; + int release_wfd; +}; + +/// Tid of the thread that burned CPU, and the burn's own result. +struct BusyReport { + long tid; +}; + +/// Fields of a /proc//stat line, indexed from 1 (field 1 is the pid). +/// Linux keeps the command in field 2 inside parentheses, so the split happens +/// after the closing one. +std::vector ParseStatFields(const std::string& text) { + std::vector fields; + const size_t open = text.find('('); + const size_t close = text.rfind(')'); + if (open == std::string::npos || close == std::string::npos || close < open) { + return fields; + } + fields.push_back(Trim(text.substr(0, open))); + fields.push_back(text.substr(open + 1, close - open - 1)); + size_t pos = close + 1; + while (pos < text.size()) { + while (pos < text.size() && (text[pos] == ' ' || text[pos] == '\n' || text[pos] == '\0')) { + ++pos; + } + size_t end = pos; + while (end < text.size() && text[end] != ' ' && text[end] != '\n' && text[end] != '\0') { + ++end; + } + if (end > pos) { + fields.push_back(text.substr(pos, end - pos)); + } + pos = end; + } + return fields; +} + +/// One numeric field of a /proc//stat line, or -1 when the file cannot be +/// read or the field is missing. +long StatFieldOf(const std::string& path, size_t index) { + std::string text; + int err = 0; + if (!ReadWholePath(path, &text, &err)) { + return -1; + } + const std::vector fields = ParseStatFields(text); + if (index == 0 || index > fields.size()) { + return -1; + } + return strtol(fields[index - 1].c_str(), nullptr, 10); +} + +/// Gives the calling thread a files table of its own with one descriptor +/// punched out of it. Only close_range(CLOSE_RANGE_UNSHARE) hands a single +/// thread a private table, so this is the setup the hidden-tid fd paths have to +/// follow. +void* PrivateTableWorker(void* arg) { + ProbePayload* payload = static_cast(arg); + const PrivateTableArg* setup = static_cast(payload->arg); + PrivateTableReport report = {}; + report.tid = GetTid(); + const long unshared = syscall(SYS_close_range, setup->punch_fd, setup->punch_fd, + kCloseRangeUnshare); + report.unshare_errno = (unshared == 0) ? 0 : errno; + // This thread's table is private now, so it holds its own copy of the + // release write end: leaving it open would keep the park below from seeing + // the owner release the pipe. + close(setup->release_wfd); + WriteRaw(payload->report_wfd, &report, sizeof(report)); + ParkUntilReleased(payload->release_rfd); + return nullptr; +} + +/// Whether `/proc//stat` and `/proc//stat` report the same CPU time. +struct PrivateRootReport { + long tid; + int unshare_errno; + int chroot_errno; +}; + +/// Gives the calling thread its own fs_struct, changes its root, then parks. +void* PrivateRootWorker(void* arg) { + ProbePayload* payload = static_cast(arg); + PrivateRootReport report = {}; + report.tid = GetTid(); + const long unshared = syscall(SYS_unshare, kCloneFs); + report.unshare_errno = (unshared == 0) ? 0 : errno; + if (unshared == 0) { + report.chroot_errno = ChrootAway() ? 0 : ENOENT; + } + WriteRaw(payload->report_wfd, &report, sizeof(report)); + ParkUntilReleased(payload->release_rfd); + return nullptr; +} + +long long MonotonicMs() { + struct timespec ts = {}; + clock_gettime(CLOCK_MONOTONIC, &ts); + return static_cast(ts.tv_sec) * 1000 + ts.tv_nsec / 1000000; +} + +/// Burns user time in this thread, which is not the group leader, then parks. +void* BusyWorker(void* arg) { + ProbePayload* payload = static_cast(arg); + BusyReport report = {}; + report.tid = GetTid(); + const long long started = MonotonicMs(); + volatile unsigned long long sink = 1; + while (MonotonicMs() - started < 300) { + for (int i = 0; i < 100000; ++i) { + sink = sink * 6364136223846793005ULL + 1442695040888963407ULL; + } + } + WriteRaw(payload->report_wfd, &report, sizeof(report)); + ParkUntilReleased(payload->release_rfd); + return nullptr; +} + +} // namespace + +// A thread that took its own files table with close_range(CLOSE_RANGE_UNSHARE) +// can hold a descriptor set the group leader does not. Linux resolves +// /proc//fd and fdinfo through get_proc_task(inode), so the thread's own +// directory reports what it closed and the leader's still reports the +// descriptor it kept. +TEST(ProcfsTaskSemantics, TidFdSubtreeUsesTheThreadsFilesTable) { + const pid_t pid = getpid(); + // The descriptor the thread punches out of its own copy. The leader keeps + // it, which is what makes the two views tell each other. + UniqueFd punch(open("/proc/version", O_RDONLY)); + ASSERT_TRUE(punch.valid()) << "cannot open the descriptor to punch: errno=" << errno; + + ProbeThread probe; + ASSERT_TRUE(probe.Prepare()) << "cannot create the probe pipes: errno=" << errno; + PrivateTableArg setup = {punch.get(), probe.release_write_fd()}; + ASSERT_TRUE(probe.Launch(PrivateTableWorker, &setup)) + << "pthread_create failed: errno=" << errno; + + PrivateTableReport report = {}; + ASSERT_TRUE(probe.ReadReport(&report, sizeof(report))) + << "the probe thread did not report: errno=" << errno; + ASSERT_EQ(0, report.unshare_errno) + << "close_range(CLOSE_RANGE_UNSHARE) failed: errno=" << report.unshare_errno; + + const std::string fd_name = std::to_string(punch.get()); + const std::string group_fd_dir = StatusPath(pid, "fd"); + const std::string thread_fd_dir = TidPath(report.tid, "fd"); + + // The leader still holds the descriptor, so the thread really took a copy + // of the table instead of closing the group's descriptor. + EXPECT_TRUE(Contains(ListDir(group_fd_dir), fd_name)) + << group_fd_dir << " lost fd " << fd_name << ": the thread closed the group's table"; + + // The thread's directory is the one that must not list it. + EXPECT_FALSE(Contains(ListDir(thread_fd_dir), fd_name)) + << thread_fd_dir << " lists fd " << fd_name << ", which the thread removed from its table"; + + char link_target[256] = {0}; + const std::string link_path = TidPath(report.tid, ("fd/" + fd_name).c_str()); + const ssize_t link_len = readlink(link_path.c_str(), link_target, sizeof(link_target) - 1); + const int link_errno = errno; + EXPECT_LT(link_len, 0) << link_path << " resolved fd " << fd_name << " of the group's table to " + << std::string(link_target, sizeof(link_target)); + EXPECT_EQ(ENOENT, link_errno) << link_path << ": errno=" << strerror(link_errno); + + // fdinfo resolves through the same table: the entry is gone for the thread + // and still there for the leader. + UniqueFd thread_fdinfo( + open(TidPath(report.tid, ("fdinfo/" + fd_name).c_str()).c_str(), O_RDONLY)); + EXPECT_FALSE(thread_fdinfo.valid()) + << "the thread's fdinfo resolved fd " << fd_name << ", which it removed from its table"; + UniqueFd group_fdinfo(open(StatusPath(pid, ("fdinfo/" + fd_name).c_str()).c_str(), O_RDONLY)); + EXPECT_TRUE(group_fdinfo.valid()) + << "the leader lost its own fdinfo entry: errno=" << errno; +} + +// Observes /proc//mounts from a thread with its own root. Runs in a forked +// child: if the guest refused to unshare the fs_struct, chroot() would move the +// whole suite's root, which must not be allowed to happen in the test process. +int CheckTidMountView() { + ProbeThread probe; + if (!probe.Prepare() || !probe.Launch(PrivateRootWorker, nullptr)) { + return 1; + } + + PrivateRootReport report = {}; + const bool reported = probe.ReadReport(&report, sizeof(report)); + int result = 1; + if (reported && report.unshare_errno == 0 && report.chroot_errno == 0) { + const std::string leader_path = "/proc/" + std::to_string(getpid()) + "/mounts"; + const std::string tid_path = "/proc/" + std::to_string(report.tid) + "/mounts"; + std::string leader; + std::string thread_view; + int err = 0; + if (ReadWholePath(leader_path, &leader, &err) && ReadWholePath(tid_path, &thread_view, &err)) { + // A non-empty leader record keeps the comparison from being + // vacuous; only the thread changed its root, so the two must differ. + result = (leader.empty() || thread_view == leader) ? 2 : 0; + } + } + // The probe owns the pipes and the join, so returning releases the parked + // thread even when an early branch gave up on the comparison. + return result; +} + +// /proc//mounts renders from the task the proc inode names: a thread that +// unshared its fs_struct and changed root must not report the group leader's +// view. Linux mounts_open_common() pins get_proc_task(inode), not the leader. +TEST(ProcfsTaskSemantics, TidMountsUsesTheThreadsRoot) { + const pid_t child = fork(); + ASSERT_GE(child, 0) << "fork failed: errno=" << errno; + if (child == 0) { + _exit(CheckTidMountView()); + } + ReapedChild child_guard(child); + + int status = 0; + bool reaped = false; + for (int i = 0; i < kPollTimeoutMs / 10; ++i) { + if (waitpid(child, &status, WNOHANG) == child) { + reaped = true; + break; + } + usleep(10000); + } + ASSERT_TRUE(reaped) << "the child did not finish"; + child_guard.Disarm(); + ASSERT_TRUE(WIFEXITED(status)) << "the child did not exit normally"; + + const int code = WEXITSTATUS(status); + if (code == 1) { + GTEST_SKIP() << "the guest cannot give a thread its own root"; + } + EXPECT_EQ(0, code) << "a thread's /proc//mounts reported the group leader's view"; +} + +// `/proc//stat` is proc_tgid_stat() even when `nr` names a non-leader +// thread, so its CPU time (14/15) aggregates the thread group the same way its +// fault counters (10/12) do. Reading one task group through two directories +// must not produce two different totals. +TEST(ProcfsTaskSemantics, TidStatReportsThreadGroupUsage) { + const pid_t pid = getpid(); + ProbeThread probe; + ASSERT_TRUE(probe.Prepare()) << "cannot create the probe pipes: errno=" << errno; + ASSERT_TRUE(probe.Launch(BusyWorker, nullptr)) << "pthread_create failed: errno=" << errno; + + BusyReport report = {}; + ASSERT_TRUE(probe.ReadReport(&report, sizeof(report))) + << "the busy thread did not report: errno=" << errno; + ASSERT_GT(report.tid, 0L); + ASSERT_NE(report.tid, GetTid()) << "the probe is not a separate thread"; + + // The group leader has run the whole suite while the probe thread burned + // 300ms of user time, so the per-thread and thread-group totals differ by + // far more than the tick granularity of the two reads. + const long group_utime = StatFieldOf(StatusPath(pid, "stat"), 14); + const long tid_utime = StatFieldOf(TidPath(report.tid, "stat"), 14); + const long thread_utime = StatFieldOf(TaskStatusPath(pid, report.tid, "stat"), 14); + ASSERT_GE(group_utime, 0L) << "cannot read /proc//stat"; + ASSERT_GE(tid_utime, 0L) << "cannot read /proc//stat"; + ASSERT_GE(thread_utime, 0L) << "cannot read /proc//task//stat"; + + EXPECT_NEAR(static_cast(group_utime), static_cast(tid_utime), 3.0) + << "the hidden tid path reported utime=" << tid_utime + << " while the group leader reported " << group_utime + << ": the record mixed per-thread CPU time with thread-group accounting"; + EXPECT_GT(thread_utime, 0L) << "the thread that burned CPU reported none"; + EXPECT_LE(thread_utime, tid_utime + 3) + << "the per-thread view " << thread_utime << " is above the group total " << tid_utime; +} + +// /proc/net/arp is served one entry per slice (Linux arp_seq_ops), so a reader +// that takes the record one byte at a time must reassemble it exactly, header +// included. A renderer that treated every slice as the first would repeat the +// header; one that dropped its cursor would lose entries. +// +// A guest without a network device renders the header alone, so what this pins +// there is the slicing contract (one header, byte-exact reassembly) rather than +// a populated table. +TEST(ProcfsTaskSemantics, ArpChunkedReadReassemblesTheSameRecord) { + std::string whole; + int err = 0; + ASSERT_TRUE(ReadWholePath("/proc/net/arp", &whole, &err)) + << "cannot read /proc/net/arp: errno=" << err; + ASSERT_FALSE(whole.empty()) << "/proc/net/arp produced no record"; + + UniqueFd fd(open("/proc/net/arp", O_RDONLY)); + ASSERT_TRUE(fd.valid()) << "cannot open /proc/net/arp: errno=" << errno; + std::string chunked; + ASSERT_EQ(0, ReadToEof(fd.get(), 1, &chunked)) << "single-byte read failed: errno=" << errno; + EXPECT_EQ(whole, chunked) << "the single-byte read did not reassemble the record"; + + const std::string header = + "IP address HW type Flags HW address Mask Device\n"; + ASSERT_GE(chunked.size(), header.size()) << "the record is shorter than its header"; + EXPECT_EQ(header, chunked.substr(0, header.size())); + EXPECT_EQ(std::string::npos, chunked.find(header, header.size())) + << "the header was rendered again on a later slice"; +} + int main(int argc, char** argv) { if (argc >= 3 && strcmp(argv[1], kMapsExecParkArg) == 0) { const int report_fd = atoi(argv[2]); From 0f94b8e70d66a4b922e49040369e041b1979106b Mon Sep 17 00:00:00 2001 From: longjin Date: Wed, 16 Sep 2026 04:10:10 +0000 Subject: [PATCH 4/7] fix(build): raise the kernel crate recursion limit for its deep auto-trait graph The kernel's object graph is one strongly-connected component, so proving `Send`/`Sync` for a type such as `File` walks an obligation chain of roughly 130 levels through it. That chain is just past rustc's default budget of 128: once `/proc/net/arp` began pinning its network namespace per file (the `seq_open_net()` behaviour asked for in review), the riscv64 and loongarch64 builds failed with error[E0275]: overflow evaluating the requirement `alloc::sync::Weak: Sync` at `kernel/src/filesystem/page_cache/writeback.rs:4393`, while x86_64 still passed. `writeback.rs` is untouched by this branch, so the report is about the solver's budget, not about a type that is actually unsound. Adding only the new `ProcfsFilePrivateData.net_ns` field on top of the previous commit reproduces that exact diagnostic, so this branch's new edge is what consumed the last of the margin; master already resolves at a depth of 121-128 on riscv64. The budget is load-bearing rather than cosmetic as well: the generated code measurably differs between budgets (the kernel crate's `.text` grows by about 2.6% from 128 to 256), so it cannot simply be ignored. Set the limit to 256, the value rustc suggests for this diagnostic and about twice the measured depth, rather than dropping an edge from the type graph or giving up the per-file namespace pin that `seq_open_net()` requires. Verified: riscv64 and loongarch64 `cargo build --release` with the CI flags and x86_64 `make kernel` all succeed, and `FMT_CHECK=1 make fmt` is clean. Signed-off-by: longjin --- kernel/src/lib.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/kernel/src/lib.rs b/kernel/src/lib.rs index 59ca47e5dd..99ff3d4fd8 100644 --- a/kernel/src/lib.rs +++ b/kernel/src/lib.rs @@ -17,6 +17,18 @@ #![feature(sync_unsafe_cell)] #![feature(linkage)] #![feature(panic_can_unwind)] +// The kernel's object graph is one strongly-connected component, so proving +// `Send`/`Sync` for a type such as `File` walks an obligation chain of roughly +// 130 levels through it -- just past rustc's default budget of 128, which +// overflowed as `E0275` on riscv64 and loongarch64 once `/proc/net/arp` began +// pinning its network namespace per file. The budget is load-bearing rather +// than cosmetic: the generated code measurably differs between budgets (the +// kernel crate's `.text` grows by ~2.6% from 128 to 256), so it is not a cap +// that is never reached. 256 is the value rustc suggests for this diagnostic +// and sets the budget to about twice the measured depth. If it ever overflows +// again, measure the new depth and raise the limit; do not drop an edge from +// the type graph to fit the budget. +#![recursion_limit = "256"] #![allow( static_mut_refs, non_local_definitions, From ac8475d7019008d2d5aa67e42c7bb517772e3c23 Mon Sep 17 00:00:00 2001 From: longjin Date: Fri, 18 Sep 2026 17:28:58 +0000 Subject: [PATCH 5/7] fix(procfs): publish the task mount view and fs context as one snapshot /proc//{mounts,mountinfo,mountstats} pin the target's mount namespace and root at open time, but `MountView::capture()` read the two slots in two separate critical sections, while a mount namespace switch published them one after the other (`PreparedNamespaceInstall::commit()` installed the prepared `fs` first and then the `nsproxy`). A reader racing `setns(CLONE_NEWNS)` or `unshare(CLONE_NEWNS)` could therefore pin the mount namespace of one generation together with the root of the next and render paths and topology from two different namespaces. Linux takes both under one `task_lock()` in `mounts_open_common()` (`get_mnt_ns()` plus `get_fs_root(task->fs, ...)`), so this is the same guarantee. Add `ProcessControlBlock::namespace_state()`, which reads `nsproxy` and the `fs` slot inside the same `task_lock` section, and make `install_prepared_namespace_state()` publish a prepared `fs` slot together with the `nsproxy` it belongs to. `prepare()` already guarantees that a mount namespace switch always carries an `fs` to publish, so no path that switches the mount namespace can publish half of the pair. The replaced `FsStruct` is handed back to the caller and dropped only after `fs_slot_update_lock` and the fs reference guard are released, because its path-pin destructors may enqueue deferred cleanup work, and `fs_slot_update_lock` is now taken only when the publication really replaces the slot, so a publication that only re-points the `nsproxy` no longer serializes with unrelated work. Guarded by `ProcfsTaskSemantics.MountViewIsOneNamespaceGeneration`, which has a child keep calling `unshare(CLONE_NEWNS)` under a reader of its `/proc//mountinfo` and fails on any record that mixes two generations. Signed-off-by: longjin --- kernel/src/filesystem/procfs/mount/collect.rs | 298 ++++++++++++---- .../procfs/mount/inode/pid_mount.rs | 22 +- kernel/src/filesystem/procfs/mount/mod.rs | 2 +- kernel/src/filesystem/procfs/mount/render.rs | 96 +++++- kernel/src/filesystem/procfs/mount/view.rs | 24 +- kernel/src/filesystem/procfs/utils.rs | 3 +- kernel/src/filesystem/vfs/mount/mod.rs | 9 + kernel/src/process/namespace/nsproxy.rs | 26 +- kernel/src/process/task.rs | 82 ++++- .../suites/normal/procfs_task_semantics.cc | 319 +++++++++++++++++- 10 files changed, 781 insertions(+), 100 deletions(-) diff --git a/kernel/src/filesystem/procfs/mount/collect.rs b/kernel/src/filesystem/procfs/mount/collect.rs index f96255faf9..618a6f563e 100644 --- a/kernel/src/filesystem/procfs/mount/collect.rs +++ b/kernel/src/filesystem/procfs/mount/collect.rs @@ -4,7 +4,7 @@ use system_error::SystemError; use crate::{ filesystem::vfs::{ - mount::{append_comma_options, MountFSInode, MountSnapshotGuard}, + mount::{append_comma_options, with_topology_snapshot, MountFSInode, MountSnapshotGuard}, FileSystem, MountFS, }, libs::casting::DowncastArc, @@ -12,12 +12,30 @@ use crate::{ use super::MountView; +/// One mount the table may list, identified without building its record. +/// +/// The table is ordered by mount id and a reader resumes it from an id, so a +/// slice still has to enumerate every mount above the cursor to know which ones +/// follow. Keeping a mount that a slice does not render down to identity is +/// what makes that enumeration a topology step instead of a whole record: the +/// fields of a mount are built by [`ProcMountEntry::resolve()`], and only for +/// the mounts the slice hands out. +#[derive(Debug)] +pub(crate) struct ProcMountCandidate { + /// Mount id: the key the table order and the reader's cursor use. + pub mount_id: usize, + pub mount: Arc, +} + +/// One record of the table, with the fields its renderer needs. #[derive(Debug)] pub(crate) struct ProcMountEntry { pub mount: Arc, pub mountpoint_display: String, pub mountinfo_root: String, pub parent_mount_id: usize, + /// Keeps the superblock backend alive while the record is rendered, without + /// making an ordinary umount report the mount busy. pub _lifecycle_pin: MountSnapshotGuard, pub mount_id: usize, pub fstype: String, @@ -26,26 +44,116 @@ pub(crate) struct ProcMountEntry { pub mountinfo_tags: String, } -pub(crate) fn collect_visible_mounts( - view: &MountView, -) -> Result<(Vec, String), SystemError> { - let root = view.root.clone(); +/// One mount of the table as the current topology shows it: the mount point and +/// root path it is rendered from, and the pin that keeps its superblock behind +/// them. +/// +/// This is what a record needs from the mount topology, so it is resolved as +/// one piece while that snapshot is held. The fields left over are built from +/// it afterwards, because they run filesystem code: see +/// [`ProcMountEntry::resolve()`]. +struct VisibleMount { + mount: Arc, + mountpoint_display: String, + parent_mount_id: usize, + mountinfo_root: String, + /// Keeps the superblock backend alive while the rest of the record is + /// built from it, without making an ordinary umount report the mount busy. + pin: MountSnapshotGuard, +} - if root.is_disconnected() { - return Ok((Vec::new(), "/".to_string())); +impl ProcMountEntry { + /// Resolves the record of `candidate`, or `None` when the mount is not part + /// of `view`'s table. + /// + /// The mount's place in the topology -- the paths it is rendered from and + /// the pin its superblock needs -- is taken under one topology snapshot per + /// record, the way `seq_path_root()` renders a path from the topology of the + /// call it serves. The rest of the record is built with that snapshot + /// released, because it runs filesystem code (the source name and the extra + /// mount options of the filesystem, then the metadata of the mount root, + /// which reads the on-disk inode of a disk filesystem): a filesystem that + /// needs the topology lock itself must not be called under it, and a slow + /// one must not stall the mount lifecycle lock every mount in the system + /// shares. + pub(crate) fn resolve( + candidate: &ProcMountCandidate, + view: &MountView, + ) -> Result, SystemError> { + let Some(visible) = with_topology_snapshot(|| VisibleMount::resolve(candidate, view))? + else { + return Ok(None); + }; + Ok(Some(Self::from_visible(visible)?)) } - let root_mount = root.mount_fs(); - let mount_namespace = view.ns.clone(); - let mut mounts = Vec::new(); - let mount_root = root_mount - .root_inode() - .downcast_arc::() - .ok_or(SystemError::EINVAL)?; - // Mirror seq_path_root(): a containing mount whose root lies above a - // chrooted ordinary directory is not visible and must not be - // synthesized as '/'. When the chroot is exactly a mount root, keep - // its real (possibly invisible) parent mount id. - if let Some(mountpoint_display) = mount_root.relative_path_from_snapshot(&root)? { + + /// Builds the fields every mount-family record shares, from a mount whose + /// superblock the snapshot pin already keeps alive. + fn from_visible(visible: VisibleMount) -> Result { + let VisibleMount { + mount, + mountpoint_display, + parent_mount_id, + mountinfo_root, + pin, + } = visible; + let mount_flags = mount.mount_flags(); + let mut per_mount_options = mount_flags.proc_rw_token().to_string(); + append_comma_options(&mut per_mount_options, mount_flags.proc_per_mount_options()); + let super_block_flags = mount.super_block_flags(); + let mut super_block_options = super_block_flags.proc_rw_token().to_string(); + append_comma_options( + &mut super_block_options, + super_block_flags.proc_super_block_options(), + ); + Ok(Self { + mount_id: mount.mount_id().into(), + fstype: mount.fs_type().to_string(), + mountinfo_tags: mount.propagation().proc_mountinfo_tags(), + per_mount_options, + super_block_options, + mount, + mountpoint_display, + mountinfo_root, + parent_mount_id, + _lifecycle_pin: pin, + }) + } +} + +impl VisibleMount { + /// Resolves `candidate` from `view`, or `None` when the mount is not part + /// of `view`'s table. The caller holds the topology snapshot. + fn resolve( + candidate: &ProcMountCandidate, + view: &MountView, + ) -> Result, SystemError> { + if Arc::ptr_eq(&candidate.mount, &view.root.mount_fs()) { + Self::resolve_pinned_root(view) + } else { + Self::resolve_attached(candidate, view) + } + } + + /// The record source of the mount the pinned root lives on. + /// + /// That mount is rendered from its root inode rather than from a mount + /// point, and its parent is the namespace's hidden root parent when it is + /// the namespace root. Mirror seq_path_root(): a containing mount whose root + /// lies above a chrooted ordinary directory is not visible and must not be + /// synthesized as '/'. When the chroot is exactly a mount root, keep its + /// real (possibly invisible) parent mount id. + fn resolve_pinned_root(view: &MountView) -> Result, SystemError> { + let root = view.root.clone(); + let root_mount = root.mount_fs(); + let root_mount_inode = root_mount + .root_inode() + .downcast_arc::() + .ok_or(SystemError::EINVAL)?; + let Some(mountpoint_display) = root_mount_inode.relative_path_from_snapshot(&root)? else { + return Ok(None); + }; + let mount_namespace = view.ns.clone(); let parent_mount_id = root_mount .self_mountpoint() .map(|mountpoint| mountpoint.mount_fs().mount_id().into()) @@ -55,60 +163,122 @@ pub(crate) fn collect_visible_mounts( .flatten() }) .unwrap_or_else(|| root_mount.mount_id().into()); - mounts.push(( + let mountinfo_root = root_mount.root_path_from_snapshot()?; + Ok(Some(Self::from_mount( + root_mount, mountpoint_display, - root_mount.root_path_from_snapshot()?, parent_mount_id, - root_mount.clone(), - )); + mountinfo_root, + )?)) } - let mut pending = root_mount.mount_children(); - while let Some(mount) = pending.pop() { - let mountpoint = mount.self_mountpoint().ok_or(SystemError::EINVAL)?; - let Some(mountpoint_display) = mountpoint.relative_path_from_snapshot(&root)? else { - continue; + + /// The record source of a mount attached below the pinned root's mount. + /// + /// Returns `None` when the mount point is outside the pinned root: a mount + /// a chroot does not reach is not part of the table, the way `seq_path_root()` + /// makes `show_vfsmnt()` emit no record for it. + fn resolve_attached( + candidate: &ProcMountCandidate, + view: &MountView, + ) -> Result, SystemError> { + let mount = candidate.mount.clone(); + // A mount the walk reached is attached to the topology, and the walk is + // re-taken per record, so a mount an umount took out of the pinned root + // in between is passed over: it is no longer part of the table either. + let Some(mountpoint) = mount.self_mountpoint() else { + return Ok(None); + }; + let Some(mountpoint_display) = mountpoint.relative_path_from_snapshot(&view.root)? else { + return Ok(None); }; let parent_mount_id = mountpoint.mount_fs().mount_id().into(); - pending.extend(mount.mount_children()); - mounts.push(( + let mountinfo_root = mount.root_path_from_snapshot()?; + Ok(Some(Self::from_mount( + mount, mountpoint_display, - mount.root_path_from_snapshot()?, parent_mount_id, + mountinfo_root, + )?)) + } + + /// The paths of `mount` plus the pin its record is built on. + fn from_mount( + mount: Arc, + mountpoint_display: String, + parent_mount_id: usize, + mountinfo_root: String, + ) -> Result { + // The mount is in the topology of this snapshot, so its superblock is + // still alive: a mount leaves the topology before it releases its claim + // on the superblock, and the `try_pin_snapshot()` failure arm is + // defensive. + let pin = mount.try_pin_snapshot()?; + Ok(Self { mount, - )); + mountpoint_display, + parent_mount_id, + mountinfo_root, + pin, + }) + } +} + +/// The mounts reachable from `view`'s pinned root, in mount-id order, above +/// `after`. +/// +/// `after` is the mount id a reader's previous slice reached: the mounts at or +/// below it keep their place in the table but no longer need their record +/// built, so a reader that resumes late does not pay again for the records it +/// already has. A sliced table is re-derived rather than resumed, because a +/// mount id is the only order the namespace offers; see +/// [`render_mount_slice()`](super::render_mount_slice) for how a slice turns the +/// result into records. +pub(crate) fn collect_mount_candidates( + view: &MountView, + after: Option, +) -> Result, SystemError> { + let root = view.root.clone(); + + if root.is_disconnected() { + return Ok(Vec::new()); + } + let root_mount = root.mount_fs(); + let mut candidates = Vec::new(); + let mut pending = Vec::new(); + push_candidate(&mut candidates, &root_mount, after); + pending.extend(root_mount.mount_children()); + + while let Some(mount) = pending.pop() { + push_candidate(&mut candidates, &mount, after); + // The mount a chroot hides still has to be walked: a visible mount + // below it is reachable through it, and only the record says whether + // the mount point is inside the pinned root. + pending.extend(mount.mount_children()); } - mounts.sort_by_key(|(_, _, _, mfs)| { - let mount_id: usize = mfs.mount_id().into(); - mount_id + // A mount is always older than the mounts below it, but a mount created + // inside one subtree is older than a sibling subtree's mount, so the tree + // gives every mount once without ordering them by id. + candidates.sort_unstable_by_key(|candidate| candidate.mount_id); + Ok(candidates) +} + +/// Adds `mount` to `candidates` unless a previous slice already reached it. +/// +/// Mount ids are allocated in increasing order and never reused +/// (`MountId::alloc()`), so "already reached" is exactly "id at or below the +/// cursor". +fn push_candidate( + candidates: &mut Vec, + mount: &Arc, + after: Option, +) { + let mount_id: usize = mount.mount_id().into(); + if after.is_some_and(|reached| mount_id <= reached) { + return; + } + candidates.push(ProcMountCandidate { + mount_id, + mount: mount.clone(), }); - let entries = mounts - .into_iter() - .map( - |(mountpoint_display, mountinfo_root, parent_mount_id, mount)| { - let mount_flags = mount.mount_flags(); - let mut per_mount_options = mount_flags.proc_rw_token().to_string(); - append_comma_options(&mut per_mount_options, mount_flags.proc_per_mount_options()); - let super_block_flags = mount.super_block_flags(); - let mut super_block_options = super_block_flags.proc_rw_token().to_string(); - append_comma_options( - &mut super_block_options, - super_block_flags.proc_super_block_options(), - ); - Ok(ProcMountEntry { - _lifecycle_pin: mount.try_pin_snapshot()?, - mount_id: mount.mount_id().into(), - fstype: mount.fs_type().to_string(), - mountinfo_tags: mount.propagation().proc_mountinfo_tags(), - per_mount_options, - super_block_options, - mount, - mountpoint_display, - mountinfo_root, - parent_mount_id, - }) - }, - ) - .collect::, SystemError>>()?; - Ok((entries, "/".to_string())) } diff --git a/kernel/src/filesystem/procfs/mount/inode/pid_mount.rs b/kernel/src/filesystem/procfs/mount/inode/pid_mount.rs index 819ce45d6c..bda6eac82f 100644 --- a/kernel/src/filesystem/procfs/mount/inode/pid_mount.rs +++ b/kernel/src/filesystem/procfs/mount/inode/pid_mount.rs @@ -2,10 +2,10 @@ use core::fmt::Debug; use crate::filesystem::{ procfs::{ - mount::{render_mount_file, MountView, ProcMountRenderKind}, + mount::{render_mount_slice, MountView, ProcMountRenderKind}, pid::ProcPidTarget, template::{Builder, FileOps, ProcFileBuilder}, - utils::proc_read_snapshot, + utils::proc_read_seq, }, vfs::{FilePrivateData, IndexNode, InodeMode}, }; @@ -46,8 +46,8 @@ impl FileOps for MountProcFileOps { // Linux `mounts_open_common()` resolves `get_proc_task(inode)` once at // open time and keeps its mount namespace and root path in the seq // private data, so a `setns()`, `unshare()` or `chroot()` performed - // afterwards cannot change what this fd reports. The record itself is - // rendered on the first read, like any other `seq_file`. + // afterwards cannot change what this fd reports. The records are + // produced by the reads, like any other `seq_file`. // // The task is the one this node names, not the group leader: a thread // can unshare its mount namespace or its `fs_struct`, and Linux then @@ -81,8 +81,18 @@ impl FileOps for MountProcFileOps { }; pdata.mount_view.clone().ok_or(SystemError::EINVAL)? }; - proc_read_snapshot(offset, len, buf, &mut data, move || { - render_mount_file(&view, self.kind) + // One slice is one output block, so an fd *keeps* a page of the table + // rather than a copy of all of it (a container may hold up to + // `mount-max` mounts and the reader up to `RLIMIT_NOFILE` fds); a slice + // still walks the namespace's mounts to find the ones that follow the + // cursor, which is work rather than retention (see + // `render_mount_slice()`). The cursor is the mount id the slice reached, + // so a mount created after an earlier slice is reported by a later one, + // the way `seq_read_iter()` re-enters `show()` per record, while a mount + // the topology took out of the pinned root in between is not reported at + // all. + proc_read_seq(offset, len, buf, &mut data, |cursor, budget, out| { + render_mount_slice(&view, self.kind, cursor, budget, out) }) } } diff --git a/kernel/src/filesystem/procfs/mount/mod.rs b/kernel/src/filesystem/procfs/mount/mod.rs index e70e26951e..c8ebb7bc73 100644 --- a/kernel/src/filesystem/procfs/mount/mod.rs +++ b/kernel/src/filesystem/procfs/mount/mod.rs @@ -9,5 +9,5 @@ pub(crate) mod inode; mod render; mod view; -pub(crate) use render::{render_mount_file, ProcMountRenderKind}; +pub(crate) use render::{render_mount_slice, ProcMountRenderKind}; pub(crate) use view::MountView; diff --git a/kernel/src/filesystem/procfs/mount/render.rs b/kernel/src/filesystem/procfs/mount/render.rs index d0b77dc82c..0f6347ebc0 100644 --- a/kernel/src/filesystem/procfs/mount/render.rs +++ b/kernel/src/filesystem/procfs/mount/render.rs @@ -5,7 +5,7 @@ use system_error::SystemError; use crate::filesystem::vfs::mount::with_topology_snapshot; use super::{ - collect::collect_visible_mounts, + collect::{collect_mount_candidates, ProcMountEntry}, fields::MountProcFields, format::{mountinfo_line, mounts_line, mountstats_line}, MountView, @@ -18,28 +18,98 @@ pub(crate) enum ProcMountRenderKind { MountStats, } -/// Renders one mount-family record (`mounts` / `mountinfo` / `mountstats`) from -/// `view`. +/// Renders one slice of the mount-family record (`mounts` / `mountinfo` / +/// `mountstats`) from `view`. /// /// Linux serves these through `seq_open_private()` (`fs/proc_namespace.c`): the /// record is produced by the reader, not at open time, so a file opened and read /// much later shows the topology the namespace reached by the time it is read — /// within the namespace and root directory `mounts_open_common()` pinned at open. -pub(crate) fn render_mount_file( +/// One visible mount is one record, and a read hands out the slice of records +/// that fits the `seq_file` buffer, so an fd holds one output block rather than +/// the whole table. +/// +/// `cursor` is the mount id the table was rendered up to (`None` starts a +/// record), and only mounts above it are reached: mount ids are allocated in +/// increasing order and never reused (`MountId::alloc()`), so a mount created +/// after an earlier slice is always reported by a later one. The id of the last +/// mount this slice reached is returned as the cursor to resume from, or `None` +/// when this slice reached the end of the table. A mount the pinned root does +/// not reach is passed over rather than rendered, and passing over it still +/// advances the cursor, the way `seq_path_root()` makes `show_vfsmnt()` return +/// without emitting a record. +/// +/// The mounts the slice does not render are still enumerated, because the mount +/// tree is what says which mounts the namespace has and the table is ordered by +/// a key only a mount carries; the walk behind +/// [`collect_mount_candidates()`] costs no record fields for them, so one slice +/// costs one enumeration of the namespace's mounts -- a walk plus a sort, taken +/// under the global mount lifecycle snapshot, and with one transient list per +/// mount it visits -- plus a record for each mount it hands out. A reader that +/// drains the whole table pays that enumeration once per slice, so its cost +/// grows with the table times the number of slices, while what the fd itself +/// keeps between reads stays one output block. +/// +/// Linux's `m_start()` resumes an iteration instead of re-deriving it, which +/// needs the ordered mount list (`ns->list`) plus the cursor node an fd keeps +/// linked into it (`fs/namespace.c`); the namespace here keeps mounts in a tree, +/// so the order is re-derived per slice. An index that would spare the walk (a +/// second, id-ordered copy of the namespace's mounts) would have to be kept in +/// step by every attach, detach, copy and propagation path, so the table pays +/// the walk instead of adding a second source of truth for what the namespace +/// contains. +/// +/// `budget` bounds the slice to roughly that many bytes: the renderer stops +/// after the record that crossed the bound, so the result is one page plus at +/// most one record, the same way `seq_read_iter()` refills `m->buf` and grows it +/// only for a single record that cannot fit. +/// +/// `out` is where the slice is appended: the caller hands in the buffer of a +/// slice that was just emptied, and the record a slice stops on stays in it +/// while the returned cursor marks the reader's place in the table. +pub(crate) fn render_mount_slice( view: &MountView, kind: ProcMountRenderKind, -) -> Result, SystemError> { - let (entries, _root_path) = with_topology_snapshot(|| collect_visible_mounts(view))?; - let mut rendered = String::new(); + cursor: Option, + budget: usize, + out: &mut Vec, +) -> Result, SystemError> { + // The candidates are enumerated from one topology snapshot, and each record + // the slice hands out resolves its own place in the topology under a + // snapshot of its own: see `ProcMountEntry::resolve()`. Only the records + // that fit the slice are resolved, so a slice covers one output block rather + // than the whole table. The enumeration itself has to stay in the snapshot: + // an edge commit such as `mount --move` publishes its two parents' mount + // point maps one after the other, so an unlocked walk could meet the same + // mount through both of them and list it twice. + let candidates = with_topology_snapshot(|| collect_mount_candidates(view, cursor))?; + let mut record = String::new(); - for entry in &entries { - let fields = MountProcFields::from_entry(entry)?; + for (index, candidate) in candidates.iter().enumerate() { + // A mount the slice passes over is still a mount the reader's table has + // reached: the cursor is the reader's place in the table, not the + // number of records it has been handed. + let Some(entry) = ProcMountEntry::resolve(candidate, view)? else { + continue; + }; + record.clear(); + let fields = MountProcFields::from_entry(&entry)?; match kind { - ProcMountRenderKind::Mounts => mounts_line::render(&fields, &mut rendered)?, - ProcMountRenderKind::MountInfo => mountinfo_line::render(&fields, &mut rendered)?, - ProcMountRenderKind::MountStats => mountstats_line::render(&fields, &mut rendered)?, + ProcMountRenderKind::Mounts => mounts_line::render(&fields, &mut record)?, + ProcMountRenderKind::MountInfo => mountinfo_line::render(&fields, &mut record)?, + ProcMountRenderKind::MountStats => mountstats_line::render(&fields, &mut record)?, + } + out.extend_from_slice(record.as_bytes()); + if out.len() >= budget { + // A record that crossed the bound is handed out with this slice; + // only a slice that has nothing left to reach ends the record. + return Ok(if index + 1 < candidates.len() { + Some(candidate.mount_id) + } else { + None + }); } } - Ok(rendered.into_bytes()) + Ok(None) } diff --git a/kernel/src/filesystem/procfs/mount/view.rs b/kernel/src/filesystem/procfs/mount/view.rs index 9c6724ddb8..ad362ac2cf 100644 --- a/kernel/src/filesystem/procfs/mount/view.rs +++ b/kernel/src/filesystem/procfs/mount/view.rs @@ -18,7 +18,12 @@ use crate::{ /// `get_fs_root(task->fs, &root)`, and stores them in the seq private data /// (`p->ns`, `p->root`, the path `seq_path_root()` renders from). A `setns()`, /// `unshare()` or `chroot()` performed afterwards therefore cannot change what -/// an already open fd reports. +/// an already open fd reports. `/proc/[pid]/mounts` is registered system-wide +/// and `/proc/mounts` and `/proc/self/mounts` resolve to it through the +/// `self` symlink. +/// +/// The two halves are also taken as one pair, because a mount namespace switch +/// publishes them together: see [`MountView::capture()`]. #[derive(Clone)] pub(crate) struct MountView { /// Mount namespace the record is collected from. @@ -40,15 +45,24 @@ impl Debug for MountView { impl MountView { /// Pins the view of `task`, which the caller resolved from the proc inode /// already (`mounts_open_common()` reports `EINVAL` for a task that is gone - /// before it gets here). + /// before it gets here), from one [`ProcessControlBlock::namespace_state()`] + /// snapshot: a `setns(CLONE_NEWNS)` or `unshare(CLONE_NEWNS)` running in + /// another thread of that task publishes a new mount namespace and a new + /// root together, and this fd must not report a mixture of the two + /// generations. /// /// A task without a root directory is `ENOENT`, like the `!task->fs` check /// there. A root that is not a mount cannot happen for one that has an /// `fs_struct`, so that arm is defensive. pub(crate) fn capture(task: &Arc) -> Result { - let ns = task.nsproxy().mnt_ns.clone(); - let root = task - .try_fs_struct() + // The mount namespace and the root are published together by every + // mount namespace switch, so they are taken together here: one + // snapshot cannot mix the namespace of one generation with the root of + // the next. + let state = task.namespace_state(); + let ns = state.nsproxy.mnt_ns.clone(); + let root = state + .fs .ok_or(SystemError::ENOENT)? .root() .downcast_arc::() diff --git a/kernel/src/filesystem/procfs/utils.rs b/kernel/src/filesystem/procfs/utils.rs index fdc25ae9a1..b6ff8adae6 100644 --- a/kernel/src/filesystem/procfs/utils.rs +++ b/kernel/src/filesystem/procfs/utils.rs @@ -94,7 +94,8 @@ impl ProcfsSeq { /// (`fs/seq_file.c:seq_read_iter()`), so a reader's read length does not decide /// how much a seq file buffers. Bounding a slice the same way keeps a large read /// from turning into a large per-fd buffer for a record source that can render -/// arbitrarily much, such as the mapping table of `/proc/[pid]/maps`. +/// arbitrarily much, such as the mapping table of `/proc/[pid]/maps` or the +/// mount table of `/proc/[pid]/mountinfo`. /// /// Only an incremental source is held to it: the sources that render one whole /// record ([`proc_read_snapshot()`], i.e. Linux `single_open()`) put that record diff --git a/kernel/src/filesystem/vfs/mount/mod.rs b/kernel/src/filesystem/vfs/mount/mod.rs index 0ec8f70ef6..8b8ca2af70 100644 --- a/kernel/src/filesystem/vfs/mount/mod.rs +++ b/kernel/src/filesystem/vfs/mount/mod.rs @@ -481,6 +481,15 @@ lazy_static! { } impl MountId { + /// Allocate the identity a mount reports in `/proc/[pid]/mountinfo`. + /// + /// Ids are handed out in strictly increasing order and are never reused, + /// which makes them a total order over the mounts that exist at any point + /// in time: a mount created later always sorts above every mount that + /// already exists. The mount tables rely on that, because an fd resumes a + /// partially read table from the id it reached. Linux gets the same effect + /// from a list position (`ns->list` plus the cursor node an fd links into + /// it) even though `ida_alloc()` may hand a freed id out again. fn alloc() -> Self { let id = NEXT_MOUNT_ID .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |id| id.checked_add(1)) diff --git a/kernel/src/process/namespace/nsproxy.rs b/kernel/src/process/namespace/nsproxy.rs index 742852df06..d674a5b5e8 100644 --- a/kernel/src/process/namespace/nsproxy.rs +++ b/kernel/src/process/namespace/nsproxy.rs @@ -423,14 +423,32 @@ impl PreparedNamespaceInstall { } else { None }; - if let Some(new_fs) = new_fs { - tsk.set_fs_struct(new_fs, &fs_refs); - } + // A mount namespace switch replaces the task's root together with its + // nsproxy, so the two are published in one task_lock section: a reader + // that snapshots them (`ProcessControlBlock::namespace_state()`) must + // not see the mount namespace of one generation with the root of the + // next. A publication that replaces the fs slot holds + // `fs_slot_update_lock` across it, so it stays serialized with an + // in-place root/pwd rewrite of the fs_struct being replaced; the + // publications that only re-point the nsproxy (`execve()`, and `setns()` + // for a namespace other than the mount namespace) have no slot to + // serialize and do not take that lock. let prepared_cred = new_cred.zip(cred_retire); - tsk.install_prepared_namespace_state(new_nsproxy, nsproxy_retire, prepared_cred); + let slot_update = new_fs.is_some().then(|| tsk.lock_fs_slot_update()); + let retired_fs = tsk.install_prepared_namespace_state( + new_nsproxy, + nsproxy_retire, + prepared_cred, + new_fs, + &fs_refs, + ); + drop(slot_update); // Keep copy-to-publication atomic against pivot_root, but do not make // unrelated fs topology writers wait for semaphore replay/wakeup work. drop(fs_refs); + // The replaced fs_struct's path-pin destructors may enqueue deferred + // cleanup work, so it is released outside every lock taken above. + drop(retired_fs); if let Some(replay) = undo_replay { replay.replay(); } diff --git a/kernel/src/process/task.rs b/kernel/src/process/task.rs index d85c4648bf..50bed2c6e2 100644 --- a/kernel/src/process/task.rs +++ b/kernel/src/process/task.rs @@ -322,6 +322,32 @@ pub struct ProcessControlBlock { pub(super) rlimits: Arc>, } +/// The namespace state of a task, taken as one pair. +/// +/// `nsproxy` and the `fs` slot describe the same generation of a task: a mount +/// namespace switch (`setns()`/`unshare(CLONE_NEWNS)`, and the same switch +/// performed by `execve()`) replaces the task's root together with its +/// `nsproxy`, so a reader that took the two slots in separate critical sections +/// could combine the old mount namespace with the new root, and +/// `/proc/[pid]/{mounts,mountinfo,mountstats}` would then render paths and +/// topology from two different namespaces. +/// +/// [`ProcessControlBlock::namespace_state()`] and +/// [`ProcessControlBlock::install_prepared_namespace_state()`] are the read and +/// write sides of that pair. The other writer of the `nsproxy` slot, +/// [`ProcessControlBlock::set_nsproxy()`], publishes it alone. Neither of its +/// callers can be observed mid-transition: a forked child rebinds its root in +/// the same fork, before the child is reachable through `/proc`, and a kernel +/// thread moved to the initial namespace already has that namespace's +/// filesystem context, because kernel threads share the kthread daemon's `fs` +/// (the daemon clones them with `CLONE_FS`, see `kthread.rs`). +pub(crate) struct TaskNamespaceState { + /// Namespace proxy the task had when the pair was taken. + pub nsproxy: Arc, + /// Filesystem context of the same instant; `None` after `exit_fs()`. + pub fs: Option>, +} + impl ProcessControlBlock { /// Create a new PCB. /// @@ -1016,9 +1042,22 @@ impl ProcessControlBlock { _fs_refs: &super::FsRefsReadGuard, ) -> Arc { let _slot_update = self.fs_slot_update_lock.lock(); + self.swap_fs_slot_locked(fs) + } + + /// Replace the fs slot with `fs` and hand the previous owner back. + /// + /// The caller must hold `fs_slot_update_lock`, so the transition is + /// serialized with an operation that rewrites the referenced `FsStruct` in + /// place. A replacement that also republishes the task's `nsproxy` (a mount + /// namespace switch) additionally holds `task_lock`: see + /// [`Self::install_prepared_namespace_state()`]. + /// + /// The previous owner is returned rather than dropped here, because its + /// path-pin destructors may enqueue deferred cleanup work. + fn swap_fs_slot_locked(&self, fs: Arc) -> Arc { let mut guard = self.fs.write(); - let old = guard.replace(fs).expect("live task must have an fs_struct"); - old + guard.replace(fs).expect("live task must have an fs_struct") } /// Drop this task's reference to its filesystem context during exit. @@ -1070,32 +1109,65 @@ impl ProcessControlBlock { self.cred.store_deferred(new); } + /// Snapshot the namespace state of this task as one coherent pair, the way + /// Linux `mounts_open_common()` reads `task->nsproxy` and `task->fs` under + /// one `task_lock()`. + /// + /// Both slots are read inside the same `task_lock` critical section that + /// publishes them ([`Self::install_prepared_namespace_state()`]), so the + /// result is exactly one generation: it never mixes the mount namespace of + /// one publication with the root of the next. + pub(crate) fn namespace_state(&self) -> TaskNamespaceState { + let _task_guard = self.task_lock.lock_irqsave(); + TaskNamespaceState { + nsproxy: self.nsproxy(), + fs: self.fs.read().clone(), + } + } + /// Publish prepared namespace state without allocating under task_lock. + /// + /// `new_fs` and `new_nsproxy` are installed in the same `task_lock` critical + /// section, because a mount namespace switch replaces the task's root and + /// its `nsproxy` as one unit: [`Self::namespace_state()`] readers must never + /// observe half of that pair. The replaced `fs` owner is returned instead of + /// being dropped here, so its path-pin destructors run after the caller + /// released the locks. + /// + /// The caller holds `fs_slot_update_lock` when `new_fs` is `Some`, which is + /// what serializes the slot transition with an in-place rewrite of the + /// replaced `FsStruct`; `_fs_refs` is not read here, and is the caller's + /// proof of the same contract as [`Self::set_fs_struct()`]: the mount + /// references of the fs context are stabilized while the slot is replaced. pub(crate) fn install_prepared_namespace_state( &self, new_nsproxy: Arc, nsproxy_retire: PreparedRcuArcRetire, new_cred: Option<(Arc, PreparedRcuArcRetire)>, - ) { + new_fs: Option>, + _fs_refs: &super::FsRefsReadGuard, + ) -> Option> { // Only credential publication needs to stabilize the active mm. Pure // namespace publication also runs inside exec, which already owns the // write side and must not recursively acquire this read lock. let _exec_guard = new_cred.as_ref().map(|_| self.exec_update_read()); let active_mm = new_cred.as_ref().and_then(|_| self.basic().user_vm()); - let (nsproxy_retirement, cred_retirement) = { + let (retired_fs, nsproxy_retirement, cred_retirement) = { let _task_guard = self.task_lock.lock_irqsave(); + let retired_fs = new_fs.map(|fs| self.swap_fs_slot_locked(fs)); let nsproxy_retirement = self.nsproxy.swap_prepared(new_nsproxy, nsproxy_retire); let cred_retirement = new_cred.map(|(cred, retire)| { self.commit_cred_side_effects(&self.cred(), &cred, active_mm.as_ref()); self.cred.swap_prepared(cred, retire) }); - (nsproxy_retirement, cred_retirement) + (retired_fs, nsproxy_retirement, cred_retirement) }; nsproxy_retirement.enqueue(); if let Some(retirement) = cred_retirement { retirement.enqueue(); } + retired_fs } #[cfg(test)] diff --git a/user/apps/tests/dunitest/suites/normal/procfs_task_semantics.cc b/user/apps/tests/dunitest/suites/normal/procfs_task_semantics.cc index d16a6f7378..8aa113ee55 100644 --- a/user/apps/tests/dunitest/suites/normal/procfs_task_semantics.cc +++ b/user/apps/tests/dunitest/suites/normal/procfs_task_semantics.cc @@ -8,7 +8,9 @@ // returning 0 even while the record grows, and moving the file position // re-renders. Before the fix every read() re-rendered and the stale byte // offset sliced the *new* render, so a second read() could hand back tail -// bytes of a longer record; +// bytes of a longer record. The mount tables below stream one mount per +// slice, so a mount created after an earlier slice is reported by a later +// one, and opening a fd pins the mount namespace and the root together; // 2. re-parenting a thread group rewrites the parent links of every thread, // so /proc//task//status and /proc//status agree on Ppid; // 3. /proc/ resolves any task that still holds a PID link (Linux @@ -27,12 +29,14 @@ #include #include #include +#include #include #include #include #include #include #include +#include #include #include #include @@ -1739,6 +1743,319 @@ TEST(ProcfsTaskSemantics, ArpChunkedReadReassemblesTheSameRecord) { << "the header was rendered again on a later slice"; } +// --------------------------------------------------------------------------- +// Guardrails for the streaming mount table +// --------------------------------------------------------------------------- + +namespace { + +/// `unshare(2)` flag that asks for a mount namespace of its own. +constexpr unsigned kCloneNewMountNs = 0x00020000; + +/// The first two fields of a mountinfo record: the id of the mount and the id +/// of the mount it is attached below. +struct MountIdPair { + unsigned long id; + unsigned long parent; +}; + +/// The id pair of the first record of `record`. +bool FirstMountIdPair(const std::string& record, MountIdPair* out) { + const size_t end = record.find('\n'); + const std::string line = (end == std::string::npos) ? record : record.substr(0, end); + return sscanf(line.c_str(), "%lu %lu", &out->id, &out->parent) == 2; +} + +/// Reads one byte of `path`, mounts a tmpfs on `dir` while that fd stays open, +/// then drains the fd. Returns 0 when the rest of the stream reports the mount +/// point, 2 when it does not, 3 when the table produced no first record, and 1 +/// when the guest cannot run the setup. +int CheckMountCreatedAfterFirstReadIsStreamed(const char* path, const std::string& dir) { + UniqueFd fd(open(path, O_RDONLY)); + if (!fd.valid()) { + return 1; + } + char first = 0; + if (ReadByte(fd.get(), &first) != 1) { + // Every namespace has a root mount, so a table that hands out no record + // at its first byte is the regression this case exists for, not an + // environment the guest cannot provide. + return 3; + } + + if (mkdir(dir.c_str(), 0755) != 0 && errno != EEXIST) { + return 1; + } + if (mount("none", dir.c_str(), "tmpfs", 0, nullptr) != 0) { + return 1; + } + + std::string rest(1, first); + const int read_errno = ReadToEof(fd.get(), kReadChunk, &rest); + const bool streamed = rest.find(dir) != std::string::npos; + umount(dir.c_str()); + rmdir(dir.c_str()); + return (read_errno == 0 && streamed) ? 0 : 2; +} + +/// Runs the check for both names the mount table is reachable under. Runs in a +/// forked child that took a mount namespace of its own, so the tmpfs the case +/// mounts cannot leak into the rest of the suite. +int CheckMountStreamChild() { + if (syscall(SYS_unshare, kCloneNewMountNs) != 0) { + return 1; + } + const int mountinfo = CheckMountCreatedAfterFirstReadIsStreamed( + "/proc/self/mountinfo", "/tmp/dunitest_mount_stream_info"); + if (mountinfo != 0) { + return mountinfo; + } + return CheckMountCreatedAfterFirstReadIsStreamed("/proc/self/mounts", + "/tmp/dunitest_mount_stream"); +} + +/// Announces `step` to the owner and waits for it to answer. +bool AnnounceAndWait(int report_wfd, int go_rfd, int step) { + return WriteRaw(report_wfd, &step, sizeof(step)) && ReadRaw(go_rfd, &step, sizeof(step)); +} + +/// How long the child below keeps taking mount namespaces before it stops on +/// its own. It is longer than the reader's window on purpose: the reader stops +/// the child by closing the pipe, and this bound only keeps a child whose owner +/// is already gone from unsharing forever. +constexpr long long kNamespaceLoopChildMs = 6000; + +/// Takes a mount namespace of its own, announces it and waits for the owner, +/// then keeps replacing its mount namespace until the owner closes the pipe or +/// the child reaches its own deadline. Returns 0 when it stopped, and the errno +/// of the step that failed otherwise. Runs in a forked child: the owner needs a +/// task that keeps moving through mount namespaces, each with the root that came +/// with it. +int CheckNamespaceLoopChild(int report_wfd, int go_rfd) { + const long long deadline = MonotonicMs() + kNamespaceLoopChildMs; + bool announced = false; + while (MonotonicMs() < deadline) { + if (syscall(SYS_unshare, kCloneNewMountNs) != 0) { + const int failed = errno; + return failed != 0 ? failed : 1; + } + if (!announced) { + announced = true; + if (!AnnounceAndWait(report_wfd, go_rfd, 1)) { + return 1; + } + } + struct pollfd release = {go_rfd, POLLIN, 0}; + if (poll(&release, 1, 0) > 0) { + return 0; + } + } + return 0; +} + +} // namespace + +// A mount created after an earlier slice of /proc//mountinfo belongs to a +// later slice: Linux renders one record per show() call, so the iteration sees +// the topology the namespace has when the reader asks for the next record. An +// fd that rendered the table once, on its first read, keeps serving that render +// and never reports the new mount point. +TEST(ProcfsTaskSemantics, MountTableStreamsMountsCreatedAfterFirstRead) { + const pid_t child = fork(); + ASSERT_GE(child, 0) << "fork failed: errno=" << errno; + if (child == 0) { + _exit(CheckMountStreamChild()); + } + ReapedChild child_guard(child); + + int status = 0; + bool reaped = false; + for (int i = 0; i < kPollTimeoutMs / 10; ++i) { + if (waitpid(child, &status, WNOHANG) == child) { + reaped = true; + break; + } + usleep(10000); + } + ASSERT_TRUE(reaped) << "the child did not finish"; + child_guard.Disarm(); + ASSERT_TRUE(WIFEXITED(status)) << "the child did not exit normally"; + + const int code = WEXITSTATUS(status); + if (code == 1) { + GTEST_SKIP() << "the guest cannot open a mount table or mount a tmpfs in a private " + "mount namespace"; + } + ASSERT_NE(3, code) << "the first read of the mount table produced no record"; + EXPECT_EQ(0, code) + << "the rest of the stream did not report a mount created after the first read"; +} + +// The read length must not decide what the fd reports: /proc//mountinfo +// hands out one slice of records per read, so a read that takes the record one +// byte at a time reassembles it exactly. A renderer that treated every slice as +// the first would repeat the leading records, and one that dropped its cursor +// would lose records. The two reads of a path are taken back to back and nothing +// else in this suite mounts in the namespace the case itself runs in (the cases +// that mount do so in a child of their own with a mount namespace of its own), +// so the record cannot move between them and the comparison can be exact. +TEST(ProcfsTaskSemantics, MountTableChunkedReadReassemblesTheSameRecord) { + const std::pair kPaths[] = { + {"/proc/self/mountinfo", true}, + {"/proc/self/mounts", true}, + {"/proc/self/mountstats", false}, + }; + for (const auto& [path, non_empty] : kPaths) { + std::string whole; + int err = 0; + ASSERT_TRUE(ReadWholePath(path, &whole, &err)) << path << ": errno=" << err; + if (non_empty) { + ASSERT_FALSE(whole.empty()) << path << " produced no record"; + } + + UniqueFd fd(open(path, O_RDONLY)); + ASSERT_TRUE(fd.valid()) << path << ": errno=" << errno; + std::string chunked; + ASSERT_EQ(0, ReadToEof(fd.get(), 1, &chunked)) << path << ": single-byte read failed"; + EXPECT_EQ(whole, chunked) << path << ": the record a read returns depends on its length"; + } +} + +// Opening the file pins the mount namespace and the root that the record is +// rendered from, and a mount namespace switch publishes the two as one unit, so +// a reader of /proc//mountinfo can only ever see a whole generation. The +// child below keeps replacing its mount namespace while the owner reads, and +// every record must look like a whole generation: a record whose first mount is +// its own parent is the signature of a capture that took the mount namespace of +// one generation and the root of the next (a namespace root is rendered with +// the id of its invisible parent). A guest that reports a namespace root as its +// own parent cannot show the difference and is skipped. +TEST(ProcfsTaskSemantics, MountViewIsOneNamespaceGeneration) { + int report[2] = {-1, -1}; + int go[2] = {-1, -1}; + ASSERT_EQ(0, pipe(report)) << "pipe failed: errno=" << errno; + ASSERT_EQ(0, pipe(go)) << "pipe failed: errno=" << errno; + + const pid_t child = fork(); + ASSERT_GE(child, 0) << "fork failed: errno=" << errno; + if (child == 0) { + close(report[0]); + close(go[1]); + _exit(CheckNamespaceLoopChild(report[1], go[0])); + } + ReapedChild child_guard(child); + close(report[1]); + close(go[0]); + + const std::string path = "/proc/" + std::to_string(child) + "/mountinfo"; + int step = 0; + if (!ReadRaw(report[0], &step, sizeof(step))) { + close(report[0]); + close(go[1]); + GTEST_SKIP() << "the child could not unshare a mount namespace"; + } + ASSERT_EQ(1, step); + std::string quiet; + int err = 0; + if (!ReadWholePath(path, &quiet, &err)) { + close(report[0]); + close(go[1]); + GTEST_SKIP() << "cannot read " << path << ": errno=" << err; + } + MountIdPair quiet_root = {}; + if (!FirstMountIdPair(quiet, &quiet_root)) { + close(report[0]); + close(go[1]); + GTEST_SKIP() << "this guest renders no mountinfo record"; + } + if (quiet_root.id == quiet_root.parent) { + close(report[0]); + close(go[1]); + GTEST_SKIP() << "this guest reports a namespace root with its own id as parent"; + } + int answer = 1; + ASSERT_TRUE(WriteRaw(go[1], &answer, sizeof(answer))); + + // An empty record is tolerated on purpose: a kernel that pins the two slots + // in two steps can leave the reader with the root of one generation and the + // mount namespace of the other, and the mounts of the pinned namespace are + // then unreachable from the pinned root, so the record comes back empty + // (Linux 6.6 does this on a measurable share of such reads). The property + // this case pins has to hold either way: a record that is not a whole + // generation is a mixture. + bool mixed = false; + std::string mixed_record; + bool read_failed = false; + int failed_errno = 0; + size_t empty_records = 0; + std::set generations; + size_t reads = 0; + const long long deadline = MonotonicMs() + 1500; + while (MonotonicMs() < deadline && reads < 20000) { + std::string record; + int read_errno = 0; + const bool readable = ReadWholePath(path, &record, &read_errno); + ++reads; + if (!readable) { + // A generation that cannot be rendered at all is a failure of its + // own: the two slots are published as one pair, so every read has a + // whole generation to render and none of them may fail. + read_failed = true; + failed_errno = read_errno; + break; + } + MountIdPair root = {}; + if (!FirstMountIdPair(record, &root)) { + // Every generation has a root mount, so a record with no mount in it + // is counted rather than ignored: a run that only ever saw those is + // not the same as one that only ever saw a single generation. + ++empty_records; + continue; + } + generations.insert(root.id); + if (root.id == root.parent) { + mixed = true; + mixed_record = record; + break; + } + } + + // Stop the child and reap it before reporting, so a failing assertion + // cannot leave it unsharing behind the case. + close(go[1]); + close(report[0]); + int status = 0; + bool reaped = false; + for (int i = 0; i < kPollTimeoutMs / 10; ++i) { + if (waitpid(child, &status, WNOHANG) == child) { + reaped = true; + break; + } + usleep(10000); + } + ASSERT_TRUE(reaped) << "the child did not stop"; + ASSERT_TRUE(WIFEXITED(status)) << "the child did not exit normally"; + // Reap before reporting and disarm the guard last, so a failed assertion + // above still kills a child that is still taking mount namespaces. + child_guard.Disarm(); + const int code = WEXITSTATUS(status); + if (code != 0) { + GTEST_SKIP() << "the child could not keep taking mount namespaces: errno=" << code; + } + + EXPECT_FALSE(read_failed) << "a read of " << path << " failed: errno=" << failed_errno; + EXPECT_FALSE(mixed) << "a read mixed the mount namespace of one generation with the root of " + "the next:\n" + << mixed_record; + if (!read_failed && !mixed && generations.size() < 2) { + // Whether the reader meets more than one generation is up to the + // scheduler, so a run that only ever saw one generation says nothing + // about the property and is not a failure of it. + GTEST_SKIP() << "the reader never saw the child change mount namespace (" << reads + << " reads, " << empty_records << " of them without a record)"; + } +} + int main(int argc, char** argv) { if (argc >= 3 && strcmp(argv[1], kMapsExecParkArg) == 0) { const int report_fd = atoi(argv[2]); From 64c91796002903183276ea540d6d8b70456ab9e1 Mon Sep 17 00:00:00 2001 From: longjin Date: Fri, 18 Sep 2026 17:29:03 +0000 Subject: [PATCH 6/7] fix(procfs): stream the mount family per slice instead of caching the table `/proc//{mounts,mountinfo,mountstats}` rendered the whole mount table into the fd on its first read and kept it there. A container with mount privileges can create close to `mount-max` (100000) mounts, and a reader can hold up to `RLIMIT_NOFILE` (1048576) descriptors, so the retained output was O(fds x mounts) of kernel heap for a reader that only needed the file to be openable. Linux serves these files through `seq_open_private()`: `seq_read_iter()` fills one `m->buf` block per call and the iterator resumes from a cursor node, so an fd keeps one output block, never the table. Do the same here: - `collect_mount_candidates()` walks the namespace's mounts from the pinned root and keeps only each mount's identity, so a mount a slice does not render does not get its record built. The table order is mount id, and `MountId::alloc()` hands ids out in increasing order and never reuses them, which is what makes "the last id this slice reached" a resume cursor. - `ProcMountEntry::resolve()` builds one record: it resolves the mount's place in the topology (mount point, root path, superblock pin) under a topology snapshot of its own, then builds the fields that run filesystem code (`proc_show_devname()`, `proc_show_mount_options()`, the mount root's `metadata()`) with that snapshot released, so a filesystem is never called under the mount lifecycle lock and a slow one cannot stall it. - `render_mount_slice()` renders the slice and returns the id to resume from; `pid_mount.rs` serves it through `proc_read_seq()`, so an fd keeps at most one page plus the single record that crossed the bound (`SEQ_SLICE_MAX`), independent of the mount count. The cursor is a watermark over the table, not a re-derivation of it: a mount the reader already passed is not handed out again, which is what Linux's cursor node does as well (it is moved past the mount in `ns->list`, and `attach_recursive_mnt()` only appends a newly attached mount to the tail, so an `MS_MOVE`, which keeps the mount's id, is not re-reported either). The walk per slice is documented on `render_mount_slice()`: the namespace keeps mounts in a tree, so the order has to be re-derived rather than resumed from a second, id-ordered index. Guarded by `MountTableStreamsMountsCreatedAfterFirstRead` (a mount created after an earlier slice is reported by a later one), `MountTableChunkedReadReassemblesTheSameRecord` (one-byte reads of all three files reassemble the whole record) and, for the pair, the namespace generation case above. Signed-off-by: longjin --- kernel/src/filesystem/procfs/mount/collect.rs | 14 ++++++++++---- kernel/src/filesystem/procfs/mount/render.rs | 14 +++++++++++++- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/kernel/src/filesystem/procfs/mount/collect.rs b/kernel/src/filesystem/procfs/mount/collect.rs index 618a6f563e..ac27ff02fc 100644 --- a/kernel/src/filesystem/procfs/mount/collect.rs +++ b/kernel/src/filesystem/procfs/mount/collect.rs @@ -182,9 +182,12 @@ impl VisibleMount { view: &MountView, ) -> Result, SystemError> { let mount = candidate.mount.clone(); - // A mount the walk reached is attached to the topology, and the walk is - // re-taken per record, so a mount an umount took out of the pinned root - // in between is passed over: it is no longer part of the table either. + // The enumeration and this record are two snapshots, so the record + // re-checks its own place instead of trusting the walk: a mount an + // umount detached, or one the pinned root no longer reaches because the + // topology moved it, is passed over -- it is not part of the table the + // reader is being handed either (Linux reaches the same result by + // letting `seq_path_root()` skip it). let Some(mountpoint) = mount.self_mountpoint() else { return Ok(None); }; @@ -267,7 +270,10 @@ pub(crate) fn collect_mount_candidates( /// /// Mount ids are allocated in increasing order and never reused /// (`MountId::alloc()`), so "already reached" is exactly "id at or below the -/// cursor". +/// cursor". A mount that becomes reachable below the cursor is deliberately +/// left out: the cursor is the reader's watermark over the table, and the +/// contract it implements is spelled out in +/// [`render_mount_slice()`](super::render_mount_slice). fn push_candidate( candidates: &mut Vec, mount: &Arc, diff --git a/kernel/src/filesystem/procfs/mount/render.rs b/kernel/src/filesystem/procfs/mount/render.rs index 0f6347ebc0..76e69c5904 100644 --- a/kernel/src/filesystem/procfs/mount/render.rs +++ b/kernel/src/filesystem/procfs/mount/render.rs @@ -39,6 +39,15 @@ pub(crate) enum ProcMountRenderKind { /// advances the cursor, the way `seq_path_root()` makes `show_vfsmnt()` return /// without emitting a record. /// +/// The cursor is a watermark over the table, not a re-derivation of it: a mount +/// whose id a slice has already reached is never handed out again, even when it +/// becomes reachable afterwards (an `MS_MOVE` that brings an existing mount +/// under the pinned root keeps that mount's id). Linux's cursor node has the +/// same shape -- `m_stop()` moves it after the last mount it emitted +/// (`fs/namespace.c`), and an `MS_MOVE` keeps the mount's place in `ns->list` +/// (`attach_recursive_mnt()` inserts only a newly attached mount at the tail), +/// so a mount the iterator already passed is likewise not reported again. +/// /// The mounts the slice does not render are still enumerated, because the mount /// tree is what says which mounts the namespace has and the table is ordered by /// a key only a mount carries; the walk behind @@ -62,7 +71,10 @@ pub(crate) enum ProcMountRenderKind { /// `budget` bounds the slice to roughly that many bytes: the renderer stops /// after the record that crossed the bound, so the result is one page plus at /// most one record, the same way `seq_read_iter()` refills `m->buf` and grows it -/// only for a single record that cannot fit. +/// only for a single record that cannot fit. The driver also bounds the slice by +/// what the reader asked for (`proc_read_seq()`), so a reader that asks for one +/// byte takes a one-record slice and pays one enumeration per record, while one +/// that asks for a page or more pays one per output block. /// /// `out` is where the slice is appended: the caller hands in the buffer of a /// slice that was just emptied, and the record a slice stops on stays in it From de9a2fe491581802c98b69c27bb0b6c61352b21e Mon Sep 17 00:00:00 2001 From: longjin Date: Fri, 18 Sep 2026 18:59:11 +0000 Subject: [PATCH 7/7] fix(procfs): keep mount records coherent within a seq slice Signed-off-by: longjin --- kernel/src/filesystem/procfs/mount/collect.rs | 114 ++++++++---------- kernel/src/filesystem/procfs/mount/render.rs | 88 +++++++++----- .../suites/normal/mount_propagation.cc | 98 ++++++++++++--- 3 files changed, 188 insertions(+), 112 deletions(-) diff --git a/kernel/src/filesystem/procfs/mount/collect.rs b/kernel/src/filesystem/procfs/mount/collect.rs index ac27ff02fc..94a47094db 100644 --- a/kernel/src/filesystem/procfs/mount/collect.rs +++ b/kernel/src/filesystem/procfs/mount/collect.rs @@ -4,7 +4,7 @@ use system_error::SystemError; use crate::{ filesystem::vfs::{ - mount::{append_comma_options, with_topology_snapshot, MountFSInode, MountSnapshotGuard}, + mount::{append_comma_options, MountFSInode, MountSnapshotGuard}, FileSystem, MountFS, }, libs::casting::DowncastArc, @@ -18,8 +18,9 @@ use super::MountView; /// slice still has to enumerate every mount above the cursor to know which ones /// follow. Keeping a mount that a slice does not render down to identity is /// what makes that enumeration a topology step instead of a whole record: the -/// fields of a mount are built by [`ProcMountEntry::resolve()`], and only for -/// the mounts the slice hands out. +/// topology fields of a mount are built by +/// [`VisibleMount::resolve_in_snapshot()`], and only for the mounts the slice +/// hands out. #[derive(Debug)] pub(crate) struct ProcMountCandidate { /// Mount id: the key the table order and the reader's cursor use. @@ -44,72 +45,45 @@ pub(crate) struct ProcMountEntry { pub mountinfo_tags: String, } -/// One mount of the table as the current topology shows it: the mount point and -/// root path it is rendered from, and the pin that keeps its superblock behind -/// them. +/// One mount of the table as one topology snapshot shows it: its paths, +/// propagation state, mount flags and superblock lifetime pin. /// -/// This is what a record needs from the mount topology, so it is resolved as -/// one piece while that snapshot is held. The fields left over are built from -/// it afterwards, because they run filesystem code: see -/// [`ProcMountEntry::resolve()`]. -struct VisibleMount { - mount: Arc, - mountpoint_display: String, - parent_mount_id: usize, - mountinfo_root: String, +/// All records in one seq slice capture this state under the same topology +/// lock. Filesystem methods and inode metadata reads run afterwards, via +/// [`ProcMountEntry::from_visible()`] and the renderer. +pub(crate) struct VisibleMount { + pub mount: Arc, + pub mountpoint_display: String, + pub parent_mount_id: usize, + pub mountinfo_root: String, + pub mount_id: usize, + pub per_mount_options: String, + pub super_block_options: String, + pub mountinfo_tags: String, /// Keeps the superblock backend alive while the rest of the record is /// built from it, without making an ordinary umount report the mount busy. - pin: MountSnapshotGuard, + pub pin: MountSnapshotGuard, } impl ProcMountEntry { - /// Resolves the record of `candidate`, or `None` when the mount is not part - /// of `view`'s table. - /// - /// The mount's place in the topology -- the paths it is rendered from and - /// the pin its superblock needs -- is taken under one topology snapshot per - /// record, the way `seq_path_root()` renders a path from the topology of the - /// call it serves. The rest of the record is built with that snapshot - /// released, because it runs filesystem code (the source name and the extra - /// mount options of the filesystem, then the metadata of the mount root, - /// which reads the on-disk inode of a disk filesystem): a filesystem that - /// needs the topology lock itself must not be called under it, and a slow - /// one must not stall the mount lifecycle lock every mount in the system - /// shares. - pub(crate) fn resolve( - candidate: &ProcMountCandidate, - view: &MountView, - ) -> Result, SystemError> { - let Some(visible) = with_topology_snapshot(|| VisibleMount::resolve(candidate, view))? - else { - return Ok(None); - }; - Ok(Some(Self::from_visible(visible)?)) - } - - /// Builds the fields every mount-family record shares, from a mount whose - /// superblock the snapshot pin already keeps alive. - fn from_visible(visible: VisibleMount) -> Result { + /// Completes a topology snapshot after releasing its lock. `fs_type()` is + /// a filesystem method and must run outside the mount lifecycle lock. + pub(crate) fn from_visible(visible: VisibleMount) -> Self { let VisibleMount { mount, mountpoint_display, parent_mount_id, mountinfo_root, + mount_id, + per_mount_options, + super_block_options, + mountinfo_tags, pin, } = visible; - let mount_flags = mount.mount_flags(); - let mut per_mount_options = mount_flags.proc_rw_token().to_string(); - append_comma_options(&mut per_mount_options, mount_flags.proc_per_mount_options()); - let super_block_flags = mount.super_block_flags(); - let mut super_block_options = super_block_flags.proc_rw_token().to_string(); - append_comma_options( - &mut super_block_options, - super_block_flags.proc_super_block_options(), - ); - Ok(Self { - mount_id: mount.mount_id().into(), + Self { + mount_id, fstype: mount.fs_type().to_string(), - mountinfo_tags: mount.propagation().proc_mountinfo_tags(), + mountinfo_tags, per_mount_options, super_block_options, mount, @@ -117,14 +91,14 @@ impl ProcMountEntry { mountinfo_root, parent_mount_id, _lifecycle_pin: pin, - }) + } } } impl VisibleMount { - /// Resolves `candidate` from `view`, or `None` when the mount is not part - /// of `view`'s table. The caller holds the topology snapshot. - fn resolve( + /// Resolve one candidate under the caller's topology snapshot. Every + /// visible mount in one seq slice is resolved in the same critical section. + pub(crate) fn resolve_in_snapshot( candidate: &ProcMountCandidate, view: &MountView, ) -> Result, SystemError> { @@ -182,12 +156,9 @@ impl VisibleMount { view: &MountView, ) -> Result, SystemError> { let mount = candidate.mount.clone(); - // The enumeration and this record are two snapshots, so the record - // re-checks its own place instead of trusting the walk: a mount an - // umount detached, or one the pinned root no longer reaches because the - // topology moved it, is passed over -- it is not part of the table the - // reader is being handed either (Linux reaches the same result by - // letting `seq_path_root()` skip it). + // Enumeration and record collection share one topology snapshot. A + // mount in the namespace tree can still be outside the pinned root; + // like Linux's `seq_path_root()`, skip a mount that root cannot reach. let Some(mountpoint) = mount.self_mountpoint() else { return Ok(None); }; @@ -216,7 +187,20 @@ impl VisibleMount { // on the superblock, and the `try_pin_snapshot()` failure arm is // defensive. let pin = mount.try_pin_snapshot()?; + let mount_flags = mount.mount_flags(); + let mut per_mount_options = mount_flags.proc_rw_token().to_string(); + append_comma_options(&mut per_mount_options, mount_flags.proc_per_mount_options()); + let super_block_flags = mount.super_block_flags(); + let mut super_block_options = super_block_flags.proc_rw_token().to_string(); + append_comma_options( + &mut super_block_options, + super_block_flags.proc_super_block_options(), + ); Ok(Self { + mount_id: mount.mount_id().into(), + mountinfo_tags: mount.propagation().proc_mountinfo_tags(), + per_mount_options, + super_block_options, mount, mountpoint_display, parent_mount_id, diff --git a/kernel/src/filesystem/procfs/mount/render.rs b/kernel/src/filesystem/procfs/mount/render.rs index 76e69c5904..f667277dd2 100644 --- a/kernel/src/filesystem/procfs/mount/render.rs +++ b/kernel/src/filesystem/procfs/mount/render.rs @@ -5,7 +5,7 @@ use system_error::SystemError; use crate::filesystem::vfs::mount::with_topology_snapshot; use super::{ - collect::{collect_mount_candidates, ProcMountEntry}, + collect::{collect_mount_candidates, ProcMountEntry, VisibleMount}, fields::MountProcFields, format::{mountinfo_line, mounts_line, mountstats_line}, MountView, @@ -18,6 +18,27 @@ pub(crate) enum ProcMountRenderKind { MountStats, } +// The shortest possible record is the `mounts` format with four empty fields: +// three field separators plus " 0 0\n". Escaping only expands path tokens, so +// their original lengths can tighten this bound without calling a filesystem. +const MIN_MOUNT_LINE_BYTES: usize = 8; + +fn minimum_line_len(mount: &VisibleMount, kind: ProcMountRenderKind) -> usize { + let path_len = mount.mountpoint_display.len(); + match kind { + ProcMountRenderKind::Mounts => MIN_MOUNT_LINE_BYTES + .saturating_add(path_len) + .saturating_add(mount.per_mount_options.len()), + ProcMountRenderKind::MountInfo => MIN_MOUNT_LINE_BYTES + .saturating_add(path_len) + .saturating_add(mount.mountinfo_root.len()) + .saturating_add(mount.per_mount_options.len()) + .saturating_add(mount.super_block_options.len()) + .saturating_add(mount.mountinfo_tags.len()), + ProcMountRenderKind::MountStats => MIN_MOUNT_LINE_BYTES.saturating_add(path_len), + } +} + /// Renders one slice of the mount-family record (`mounts` / `mountinfo` / /// `mountstats`) from `view`. /// @@ -51,13 +72,13 @@ pub(crate) enum ProcMountRenderKind { /// The mounts the slice does not render are still enumerated, because the mount /// tree is what says which mounts the namespace has and the table is ordered by /// a key only a mount carries; the walk behind -/// [`collect_mount_candidates()`] costs no record fields for them, so one slice -/// costs one enumeration of the namespace's mounts -- a walk plus a sort, taken -/// under the global mount lifecycle snapshot, and with one transient list per -/// mount it visits -- plus a record for each mount it hands out. A reader that -/// drains the whole table pays that enumeration once per slice, so its cost -/// grows with the table times the number of slices, while what the fd itself -/// keeps between reads stays one output block. +/// [`collect_mount_candidates()`] costs no record fields for them. One slice +/// enumerates and sorts the candidates, then captures enough visible mounts +/// to meet the output budget by a conservative lower bound of each formatted +/// line, under the same topology snapshot. The +/// filesystem callbacks run after releasing that snapshot. A +/// reader that drains the table pays one enumeration per slice, while the fd +/// itself keeps only one output block. /// /// Linux's `m_start()` resumes an iteration instead of re-deriving it, which /// needs the ordered mount list (`ns->list`) plus the cursor node an fd keeps @@ -86,24 +107,33 @@ pub(crate) fn render_mount_slice( budget: usize, out: &mut Vec, ) -> Result, SystemError> { - // The candidates are enumerated from one topology snapshot, and each record - // the slice hands out resolves its own place in the topology under a - // snapshot of its own: see `ProcMountEntry::resolve()`. Only the records - // that fit the slice are resolved, so a slice covers one output block rather - // than the whole table. The enumeration itself has to stay in the snapshot: - // an edge commit such as `mount --move` publishes its two parents' mount - // point maps one after the other, so an unlocked walk could meet the same - // mount through both of them and list it twice. - let candidates = with_topology_snapshot(|| collect_mount_candidates(view, cursor))?; + // Linux m_start()/m_stop() holds namespace_sem across a seq buffer's + // records. Capture the topology fields for this buffer under one snapshot: + // an MS_REC propagation change must not put different generations in two + // lines of one read. The selected Vec lives outside the closure so an error + // releases the topology lock before dropping any superblock snapshot pins. + let mut selected = Vec::new(); + let mut covered_bytes = 0usize; + let has_more = with_topology_snapshot(|| -> Result { + let candidates = collect_mount_candidates(view, cursor)?; + for (index, candidate) in candidates.iter().enumerate() { + if let Some(visible) = VisibleMount::resolve_in_snapshot(candidate, view)? { + covered_bytes = covered_bytes.saturating_add(minimum_line_len(&visible, kind)); + selected.push(visible); + if covered_bytes >= budget { + return Ok(index + 1 < candidates.len()); + } + } + } + Ok(false) + })?; + let selected_count = selected.len(); + let last_selected_id = selected.last().map(|mount| mount.mount_id); let mut record = String::new(); - for (index, candidate) in candidates.iter().enumerate() { - // A mount the slice passes over is still a mount the reader's table has - // reached: the cursor is the reader's place in the table, not the - // number of records it has been handed. - let Some(entry) = ProcMountEntry::resolve(candidate, view)? else { - continue; - }; + for (index, visible) in selected.into_iter().enumerate() { + let min_line_len = minimum_line_len(&visible, kind); + let entry = ProcMountEntry::from_visible(visible); record.clear(); let fields = MountProcFields::from_entry(&entry)?; match kind { @@ -111,17 +141,21 @@ pub(crate) fn render_mount_slice( ProcMountRenderKind::MountInfo => mountinfo_line::render(&fields, &mut record)?, ProcMountRenderKind::MountStats => mountstats_line::render(&fields, &mut record)?, } + debug_assert!(record.len() >= min_line_len); out.extend_from_slice(record.as_bytes()); if out.len() >= budget { // A record that crossed the bound is handed out with this slice; // only a slice that has nothing left to reach ends the record. - return Ok(if index + 1 < candidates.len() { - Some(candidate.mount_id) + return Ok(if index + 1 < selected_count || has_more { + Some(entry.mount_id) } else { None }); } } - Ok(None) + // If there are more candidates, the minimum line length proves the selected + // records filled the budget. This arm is defensive if a formatter changes. + debug_assert!(!has_more || out.len() >= budget); + Ok(if has_more { last_selected_id } else { None }) } diff --git a/user/apps/tests/dunitest/suites/normal/mount_propagation.cc b/user/apps/tests/dunitest/suites/normal/mount_propagation.cc index 7773b0fef3..d293e8fff7 100644 --- a/user/apps/tests/dunitest/suites/normal/mount_propagation.cc +++ b/user/apps/tests/dunitest/suites/normal/mount_propagation.cc @@ -202,6 +202,31 @@ bool parse_mountinfo_tags(char* line, const char* mount_point, PropagationTags* return token != nullptr; } +void scan_propagation_line(const char* line, const char* const* mount_points, size_t count, + PropagationTags* tags, bool* found) { + for (size_t i = 0; i < count; ++i) { + if (found[i]) { + continue; + } + char copy[2048] = {}; + const size_t line_len = strnlen(line, sizeof(copy) - 1); + memcpy(copy, line, line_len); + copy[line_len] = '\0'; + if (parse_mountinfo_tags(copy, mount_points[i], &tags[i])) { + found[i] = true; + } + } +} + +bool all_propagation_paths_found(const bool* found, size_t count) { + for (size_t i = 0; i < count; ++i) { + if (!found[i]) { + return false; + } + } + return true; +} + bool read_propagation_snapshot(const char* const* mount_points, size_t count, PropagationTags* tags) { FILE* fp = fopen("/proc/self/mountinfo", "r"); @@ -216,26 +241,51 @@ bool read_propagation_snapshot(const char* const* mount_points, size_t count, } char line[2048] = {}; while (fgets(line, sizeof(line), fp) != nullptr) { - for (size_t i = 0; i < count; ++i) { - if (found[i]) { - continue; - } - char copy[sizeof(line)] = {}; - const size_t line_len = strnlen(line, sizeof(copy) - 1); - memcpy(copy, line, line_len); - copy[line_len] = '\0'; - if (parse_mountinfo_tags(copy, mount_points[i], &tags[i])) { - found[i] = true; - } - } + scan_propagation_line(line, mount_points, count, tags, found); } fclose(fp); - for (size_t i = 0; i < count; ++i) { - if (!found[i]) { - return false; + return all_propagation_paths_found(found, count); +} + +// Linux holds namespace_sem only from m_start() to m_stop() for one seq buffer +// fill. DragonOS also takes a separate topology snapshot for each 4 KiB slice, +// even when one read() asks for more. Limit the request to one x86_64 page and +// accept it only if all three target lines are complete within that slice. +bool read_propagation_snapshot_once(const char* const* mount_points, size_t count, + PropagationTags* tags) { + bool found[8] = {}; + if (count > sizeof(found) / sizeof(found[0])) { + return false; + } + int fd = open("/proc/self/mountinfo", O_RDONLY); + if (fd < 0) { + return false; + } + constexpr size_t kMountinfoSliceBytes = 4096; + char contents[kMountinfoSliceBytes + 1] = {}; + ssize_t length; + do { + length = read(fd, contents, kMountinfoSliceBytes); + } while (length < 0 && errno == EINTR); + close(fd); + if (length <= 0) { + return false; + } + + // Only complete lines from this one seq slice count. If the target mounts + // are not all present, the test cannot claim an atomic observation. + char* line = contents; + char* const end = contents + length; + while (line < end) { + char* newline = static_cast(memchr(line, '\n', end - line)); + if (newline == nullptr) { + break; } + *newline = '\0'; + scan_propagation_line(line, mount_points, count, tags, found); + line = newline + 1; } - return true; + return all_propagation_paths_found(found, count); } bool snapshot_is_uniform(const PropagationTags* tags, size_t count) { @@ -1577,6 +1627,14 @@ TEST_F(MountPropagationTest, RecursiveChangesAreAtomicAgainstSnapshotsAndNamespa ASSERT_EQ(0, mount("", grandchild, "ramfs", 0, nullptr)) << strerror(errno); ASSERT_EQ(0, ensure_dir(dynamic)) << strerror(errno); + // This test compares three records from one seq slice. Check the fixture + // layout before workers race so an enlarged mount table has a clear error + // instead of being reported as a propagation atomicity failure. + const char* snapshot_paths[] = {base, child, grandchild}; + PropagationTags initial_tags[3] = {}; + ASSERT_TRUE(read_propagation_snapshot_once(snapshot_paths, 3, initial_tags)) + << "test mounts must fit as complete lines in the first mountinfo page"; + int start_pipe[2] = {-1, -1}; ASSERT_EQ(0, pipe(start_pipe)) << strerror(errno); int activity_pipe[2] = {-1, -1}; @@ -1718,7 +1776,7 @@ TEST_F(MountPropagationTest, RecursiveChangesAreAtomicAgainstSnapshotsAndNamespa for (size_t i = 0; i < 2; ++i) { PropagationTags tags[3] = {}; if (!read_exact(activity_pipe[0], &token, 1) || token != phase_tokens[i] || - !read_propagation_snapshot(paths, 3, tags) || !snapshot_is_uniform(tags, 3) || + !read_propagation_snapshot_once(paths, 3, tags) || !snapshot_is_uniform(tags, 3) || (tags[0].shared > 0) != phase_is_shared[i] || !write_exact(ready_pipe[1], &token, 1)) { _exit(10); @@ -1728,7 +1786,7 @@ TEST_F(MountPropagationTest, RecursiveChangesAreAtomicAgainstSnapshotsAndNamespa _exit(9); } PropagationTags midpoint_tags[3] = {}; - if (!read_propagation_snapshot(paths, 3, midpoint_tags) || + if (!read_propagation_snapshot_once(paths, 3, midpoint_tags) || !snapshot_is_uniform(midpoint_tags, 3) || !write_exact(ready_pipe[1], "X", 1)) { _exit(10); @@ -1746,7 +1804,7 @@ TEST_F(MountPropagationTest, RecursiveChangesAreAtomicAgainstSnapshotsAndNamespa _exit(9); } PropagationTags tags[3] = {}; - if (!read_propagation_snapshot(paths, 3, tags) || !snapshot_is_uniform(tags, 3)) { + if (!read_propagation_snapshot_once(paths, 3, tags) || !snapshot_is_uniform(tags, 3)) { _exit(10); } } @@ -1777,7 +1835,7 @@ TEST_F(MountPropagationTest, RecursiveChangesAreAtomicAgainstSnapshotsAndNamespa } const char* paths[] = {base, child, grandchild}; PropagationTags tags[3] = {}; - if (!read_propagation_snapshot(paths, 3, tags) || !snapshot_is_uniform(tags, 3)) { + if (!read_propagation_snapshot_once(paths, 3, tags) || !snapshot_is_uniform(tags, 3)) { _exit(13); } if (!write_exact(worker_done_pipe[1], "D", 1)) {