Skip to content

Commit 891fa32

Browse files
fslongjinfslongjin
authored
fix(vfs): use caller fs root for pivot_root (#2130)
* fix(vfs): use caller fs root for pivot_root Resolve and pin the caller fs root, new_root, and put_old as mount-aware paths so pivot_root follows Linux object identity and reachability semantics after chroot. Serialize task publication with exact root/pwd migration, keep mount and dentry topology under one guard, and pre-reserve all edge and task-tracking capacity before the infallible commit. Preserve stacked mount ordering and Linux errno precedence without visible-path approximations. Add focused dunitests for chrooted pivots, namespace-root failures, cross-process fs reference updates, stacked mounts, symlink and bind aliases, and unrelated upper mounts. Tests: make kernel Tests: test_pivot_root_test (20/20) Tests: mount_object_topology_test (9/9) Tests: mount_move_test (11/11) Tests: mount_propagation_test (24/24) Signed-off-by: longjin <longjin@dragonos.org> * fix(vfs): address pivot_root review races Restore the namespace-root pivot path while preserving DragonOS mount namespace invariants and publish the replacement root only after the topology commit is complete. Serialize fs_struct copying and publication against pivot_root migration with a reader-writer barrier covering fork, unshare, setns, and exec namespace switches. Extend pivot_root coverage to verify the standard namespace-root operation and the old-root attachment. Signed-off-by: longjin <longjin@dragonos.org> * test(vfs): cover shared namespace root pivot Distinguish the caller's shared root mount from the private parent of the new root, matching Linux pivot_root propagation checks. Keep markers on both the replacement root and the relocated old root so the test also verifies the committed topology. Signed-off-by: longjin <longjin@dragonos.org> * fix(process): release fs slot before cleanup Clear the exiting task's filesystem slot under its update lock, then release the lock before dropping the final FsStruct owner. This matches Linux exit_fs lifetime ordering and keeps path-pin and mount lifecycle cleanup outside the pivot_root slot critical section. Signed-off-by: longjin <longjin@dragonos.org> * fix(vfs): avoid pivot_root lock inversion Acquire every dentry mount gate touched by pivot_root before taking the dentry topology snapshot. Sort and deduplicate the fixed gate set, and require an unforgeable commit token for prelocked mount-edge operations so the canonical order is enforced by the API. Revalidate namespace membership, exact mount roots, connectivity, propagation constraints, and path reachability only after lifecycle, namespace, gate, and dentry locks are held. Keep all fallible reservations before the first edge mutation and preserve the topology guard across fs reference repair. This removes the ABBA cycle with unlink, rmdir, and rename, which acquire mount gates before the dentry topology writer. Signed-off-by: longjin <longjin@dragonos.org> * style(vfs): satisfy repository format checks Apply the repository rustfmt layout and remove a needless borrow reported by the deny-by-default clippy configuration. Signed-off-by: longjin <longjin@dragonos.org> * perf(process): narrow fork fs publication barrier Complete signal, address-space, architecture, and metadata copies before entering the fs reference publication barrier. Keep copy_fs and copy_namespaces adjacent, and retain the read guard through namespace-dependent PID setup and final PCB publication. Release the guard immediately after add_pcb so pivot_root still observes every copied fs_struct without making unrelated cgroup accounting and fork counters part of the global critical section. Signed-off-by: longjin <longjin@dragonos.org> --------- Signed-off-by: longjin <longjin@dragonos.org>
1 parent c6808a8 commit 891fa32

14 files changed

Lines changed: 1169 additions & 340 deletions

File tree

‎kernel/src/filesystem/fs.rs‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -166,4 +166,31 @@ impl FsStruct {
166166
pub fn root_resolved(&self) -> Result<ResolvedPath, system_error::SystemError> {
167167
self.path_context.read().root.resolved()
168168
}
169+
170+
/// Linux `chroot_fs_refs()` equivalent for one fs_struct. Both comparisons
171+
/// and replacements are performed under one path-context write lock.
172+
pub fn replace_root_pwd(&self, old: &ResolvedPath, new: &ResolvedPath) -> bool {
173+
let old_inode = old.inode();
174+
let mut paths = self.path_context.write();
175+
let root_hit = same_path_ref(&paths.root.inode, &old_inode);
176+
let pwd_hit = same_path_ref(&paths.pwd.inode, &old_inode);
177+
178+
if root_hit {
179+
paths.root = PinnedPath::from_resolved(new.derive_existing_owner());
180+
}
181+
if pwd_hit {
182+
paths.pwd = PinnedPath::from_resolved(new.derive_existing_owner());
183+
}
184+
root_hit || pwd_hit
185+
}
186+
}
187+
188+
fn same_path_ref(left: &Arc<dyn IndexNode>, right: &Arc<dyn IndexNode>) -> bool {
189+
let Some(left_mount) = left.clone().downcast_arc::<MountFSInode>() else {
190+
return Arc::ptr_eq(left, right);
191+
};
192+
let Some(right_mount) = right.clone().downcast_arc::<MountFSInode>() else {
193+
return false;
194+
};
195+
left_mount.same_path_ref(&right_mount)
169196
}

‎kernel/src/filesystem/vfs/inode_lifecycle.rs‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,14 @@ impl InodeRetentionGuard {
132132
inode.retain(kind)?;
133133
Ok(Self { inode, kind })
134134
}
135+
136+
/// Derive a new semantic owner while this owner keeps retention admission
137+
/// open. `try_begin_freeing()` can close admission only when every kind has
138+
/// a zero count, which is impossible while `self` is alive.
139+
pub(crate) fn derive_existing(&self, kind: InodeRetentionKind) -> Self {
140+
Self::new(self.inode.clone(), kind)
141+
.expect("an existing inode retention owner must remain derivable")
142+
}
135143
}
136144

137145
impl Drop for InodeRetentionGuard {

‎kernel/src/filesystem/vfs/mount/mod.rs‎

Lines changed: 223 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ use crate::{
1616
casting::DowncastArc,
1717
errseq::{ErrSeq, ErrSeqValue},
1818
mutex::{Mutex, MutexGuard},
19-
rwsem::{RwSem, RwSemWriteGuard},
19+
rwsem::{RwSem, RwSemReadGuard, RwSemWriteGuard},
2020
spinlock::SpinLock,
2121
wait_queue::WaitQueue,
2222
},
@@ -57,8 +57,9 @@ use system_error::SystemError;
5757
/// detach, including propagation peers in other namespaces.
5858
///
5959
/// Mount topology and propagation code acquires locks in this order:
60-
/// lifecycle -> namespace -> dentry mount gate -> parent mountpoints -> peer
61-
/// registry -> one mount's propagation state -> propagation group allocator.
60+
/// lifecycle -> namespace -> dentry mount gates (ordered by dentry ID) ->
61+
/// dentry topology snapshot -> parent mountpoints -> peer registry -> one
62+
/// mount's propagation state -> propagation group allocator.
6263
/// A lower layer must never acquire the lifecycle/topology layers in reverse.
6364
pub(crate) static MOUNT_LIFECYCLE_LOCK: Mutex<()> = Mutex::new(());
6465

@@ -69,6 +70,129 @@ lazy_static! {
6970
static ref DENTRY_TOPOLOGY_LOCK: RwSem<()> = RwSem::new(());
7071
}
7172

73+
/// Stable snapshot of both mount edges and dentry parent/name relationships.
74+
///
75+
/// Keep the field order in sync with the required release order: Rust drops
76+
/// fields in declaration order, so the dentry reader is released before the
77+
/// outer mount-lifecycle mutex.
78+
pub struct MountTopologyGuard {
79+
_dentries: RwSemReadGuard<'static, ()>,
80+
_mounts: MutexGuard<'static, ()>,
81+
}
82+
83+
/// First stage of a mount-topology transaction.
84+
///
85+
/// Edge commits which also need dentry mount gates use this capability to
86+
/// acquire those gates before completing the dentry topology snapshot.
87+
pub(crate) struct MountLifecycleGuard {
88+
mounts: MutexGuard<'static, ()>,
89+
}
90+
91+
/// Proof that every dentry gate used by one mount-edge commit is held.
92+
/// The fields are private so only the gate-set helper can construct it.
93+
pub(crate) struct MountEdgeCommitToken {
94+
dentries: [Option<DentryId>; 3],
95+
}
96+
97+
impl MountEdgeCommitToken {
98+
fn covers(&self, mountpoint: &MountFSInode) -> bool {
99+
let id = mountpoint.dentry.id;
100+
self.dentries.iter().flatten().any(|entry| *entry == id)
101+
}
102+
}
103+
104+
pub(crate) fn lock_mount_lifecycle() -> MountLifecycleGuard {
105+
MountLifecycleGuard {
106+
mounts: MOUNT_LIFECYCLE_LOCK.lock(),
107+
}
108+
}
109+
110+
/// Acquire the complete VFS topology snapshot in the canonical lock order.
111+
pub(crate) fn lock_mount_topology() -> MountTopologyGuard {
112+
lock_mount_lifecycle().complete()
113+
}
114+
115+
impl MountLifecycleGuard {
116+
fn complete(self) -> MountTopologyGuard {
117+
let dentries = DENTRY_TOPOLOGY_LOCK.read();
118+
MountTopologyGuard {
119+
_dentries: dentries,
120+
_mounts: self.mounts,
121+
}
122+
}
123+
124+
/// Lock every dentry gate touched by an exact edge commit before taking
125+
/// the dentry snapshot. Directory mutations hold the same gate while
126+
/// acquiring the topology writer, so waiting for a gate under a read
127+
/// snapshot would invert that order.
128+
pub(crate) fn commit_mount_edges(
129+
self,
130+
mountpoints: [Option<Arc<MountFSInode>>; 3],
131+
operation: impl FnOnce(&MountEdgeCommitToken) -> Result<(), SystemError>,
132+
) -> Result<MountTopologyGuard, SystemError> {
133+
let mounts = self.mounts;
134+
let dentries = mountpoints.map(|mountpoint| mountpoint.map(|inode| inode.dentry.clone()));
135+
with_dentry_mount_gate_set(dentries, |token| {
136+
let snapshot = DENTRY_TOPOLOGY_LOCK.read();
137+
operation(token)?;
138+
Ok(MountTopologyGuard {
139+
_dentries: snapshot,
140+
_mounts: mounts,
141+
})
142+
})
143+
}
144+
}
145+
146+
fn with_dentry_mount_gate_set<T>(
147+
mut dentries: [Option<Arc<VfsDentry>>; 3],
148+
operation: impl FnOnce(&MountEdgeCommitToken) -> T,
149+
) -> T {
150+
dentries.sort_unstable_by_key(|entry| {
151+
entry
152+
.as_ref()
153+
.map(|dentry| dentry.id.0)
154+
.unwrap_or(usize::MAX)
155+
});
156+
157+
let mut unique: [Option<Arc<VfsDentry>>; 3] = [None, None, None];
158+
let mut count = 0;
159+
for dentry in dentries.into_iter().flatten() {
160+
if count == 0
161+
|| unique[count - 1]
162+
.as_ref()
163+
.is_none_or(|previous| previous.id != dentry.id)
164+
{
165+
unique[count] = Some(dentry);
166+
count += 1;
167+
}
168+
}
169+
let token = MountEdgeCommitToken {
170+
dentries: unique
171+
.each_ref()
172+
.map(|entry| entry.as_ref().map(|dentry| dentry.id)),
173+
};
174+
175+
match count {
176+
0 => operation(&token),
177+
1 => {
178+
let _first = unique[0].as_ref().unwrap().mount_gate.lock();
179+
operation(&token)
180+
}
181+
2 => {
182+
let _first = unique[0].as_ref().unwrap().mount_gate.lock();
183+
let _second = unique[1].as_ref().unwrap().mount_gate.lock();
184+
operation(&token)
185+
}
186+
3 => {
187+
let _first = unique[0].as_ref().unwrap().mount_gate.lock();
188+
let _second = unique[1].as_ref().unwrap().mount_gate.lock();
189+
let _third = unique[2].as_ref().unwrap().mount_gate.lock();
190+
operation(&token)
191+
}
192+
_ => unreachable!(),
193+
}
194+
}
195+
72196
/// Capability for one layered directory mutation to acquire the global dentry
73197
/// topology write lock exactly once. Acquisition is deliberately lazy:
74198
/// layered filesystems may prepare a copy-up before the innermost backing
@@ -98,8 +222,7 @@ impl DentryMutationContext<'_> {
98222
}
99223

100224
pub(crate) fn with_topology_snapshot<T>(f: impl FnOnce() -> T) -> T {
101-
let _mounts = MOUNT_LIFECYCLE_LOCK.lock();
102-
let _dentries = DENTRY_TOPOLOGY_LOCK.read();
225+
let _topology = lock_mount_topology();
103226
f()
104227
}
105228

@@ -1271,11 +1394,35 @@ impl MountFS {
12711394
self.attach_top_inner(mountpoint, mount_fs, true)
12721395
}
12731396

1397+
pub(crate) fn attach_new_top_prelocked(
1398+
&self,
1399+
mountpoint: &Arc<MountFSInode>,
1400+
mount_fs: Arc<MountFS>,
1401+
gates: &MountEdgeCommitToken,
1402+
) -> Result<(), SystemError> {
1403+
assert!(
1404+
gates.covers(mountpoint),
1405+
"mount-edge commit token must cover the attach dentry"
1406+
);
1407+
self.validate_attach_top(mountpoint, &mount_fs)?;
1408+
self.attach_top_prelocked(mountpoint, mount_fs, true)
1409+
}
1410+
12741411
fn attach_top_inner(
12751412
&self,
12761413
mountpoint: &Arc<MountFSInode>,
12771414
mount_fs: Arc<MountFS>,
12781415
require_connected: bool,
1416+
) -> Result<(), SystemError> {
1417+
self.validate_attach_top(mountpoint, &mount_fs)?;
1418+
let _gate = mountpoint.dentry.mount_gate.lock();
1419+
self.attach_top_prelocked(mountpoint, mount_fs, require_connected)
1420+
}
1421+
1422+
fn validate_attach_top(
1423+
&self,
1424+
mountpoint: &Arc<MountFSInode>,
1425+
mount_fs: &Arc<MountFS>,
12791426
) -> Result<(), SystemError> {
12801427
if !Arc::ptr_eq(&mountpoint.mount_fs, &self.self_ref()) {
12811428
return Err(SystemError::EINVAL);
@@ -1287,7 +1434,15 @@ impl MountFS {
12871434
{
12881435
return Err(SystemError::EINVAL);
12891436
}
1290-
let _gate = mountpoint.dentry.mount_gate.lock();
1437+
Ok(())
1438+
}
1439+
1440+
fn attach_top_prelocked(
1441+
&self,
1442+
mountpoint: &Arc<MountFSInode>,
1443+
mount_fs: Arc<MountFS>,
1444+
require_connected: bool,
1445+
) -> Result<(), SystemError> {
12911446
if require_connected && mountpoint.is_disconnected() {
12921447
return Err(SystemError::ENOENT);
12931448
}
@@ -1554,7 +1709,7 @@ impl MountFS {
15541709
let _cover_reservation = mount_fs.reserve_mount_edge(cover_mountpoint, 0)?;
15551710
mount_fs.detach_exact_keep_slot(covered)?;
15561711
covered.relocate_mountpoint(Some(original_mountpoint.clone()));
1557-
let removed = match self.replace_exact_edge(mount_fs, covered.clone()) {
1712+
let removed = match self.replace_exact_edge_prepared(mount_fs, covered.clone()) {
15581713
Ok(removed) => removed,
15591714
Err(error) => {
15601715
// All objects remain owned; reconstruct the tuck-under
@@ -1571,21 +1726,44 @@ impl MountFS {
15711726

15721727
/// Replace one exact edge in place, preserving the parent stack's key,
15731728
/// capacity, ordering and mount-edge count.
1574-
fn replace_exact_edge(
1729+
pub(crate) fn replace_exact_edge_prepared(
15751730
&self,
15761731
old: &Arc<MountFS>,
15771732
replacement: Arc<MountFS>,
15781733
) -> Result<Arc<MountFS>, SystemError> {
15791734
let mountpoint = old.self_mountpoint().ok_or(SystemError::EINVAL)?;
1735+
let _gate = mountpoint.dentry.mount_gate.lock();
1736+
self.replace_exact_edge_prepared_prelocked(old, replacement, &mountpoint)
1737+
}
1738+
1739+
pub(crate) fn replace_exact_edge_prepared_with_token(
1740+
&self,
1741+
old: &Arc<MountFS>,
1742+
replacement: Arc<MountFS>,
1743+
gates: &MountEdgeCommitToken,
1744+
) -> Result<Arc<MountFS>, SystemError> {
1745+
let mountpoint = old.self_mountpoint().ok_or(SystemError::EINVAL)?;
1746+
assert!(
1747+
gates.covers(&mountpoint),
1748+
"mount-edge commit token must cover the replace dentry"
1749+
);
1750+
self.replace_exact_edge_prepared_prelocked(old, replacement, &mountpoint)
1751+
}
1752+
1753+
fn replace_exact_edge_prepared_prelocked(
1754+
&self,
1755+
old: &Arc<MountFS>,
1756+
replacement: Arc<MountFS>,
1757+
mountpoint: &Arc<MountFSInode>,
1758+
) -> Result<Arc<MountFS>, SystemError> {
15801759
if !Arc::ptr_eq(&mountpoint.mount_fs, &self.self_ref())
15811760
|| replacement
15821761
.self_mountpoint()
15831762
.as_ref()
1584-
.is_none_or(|replacement_mp| !Arc::ptr_eq(replacement_mp, &mountpoint))
1763+
.is_none_or(|replacement_mp| !Arc::ptr_eq(replacement_mp, mountpoint))
15851764
{
15861765
return Err(SystemError::EINVAL);
15871766
}
1588-
let _gate = mountpoint.dentry.mount_gate.lock();
15891767
let mut mountpoints = self.mountpoints.lock();
15901768
let stack = mountpoints
15911769
.get_mut(&mountpoint.dentry.id)
@@ -1602,10 +1780,31 @@ impl MountFS {
16021780
mount_fs: &Arc<MountFS>,
16031781
) -> Result<Arc<MountFS>, SystemError> {
16041782
let mountpoint = mount_fs.self_mountpoint().ok_or(SystemError::EINVAL)?;
1783+
let _gate = mountpoint.dentry.mount_gate.lock();
1784+
self.detach_exact_keep_slot_prelocked(mount_fs, &mountpoint)
1785+
}
1786+
1787+
pub(crate) fn detach_exact_keep_slot_with_token(
1788+
&self,
1789+
mount_fs: &Arc<MountFS>,
1790+
gates: &MountEdgeCommitToken,
1791+
) -> Result<Arc<MountFS>, SystemError> {
1792+
let mountpoint = mount_fs.self_mountpoint().ok_or(SystemError::EINVAL)?;
1793+
assert!(
1794+
gates.covers(&mountpoint),
1795+
"mount-edge commit token must cover the detach dentry"
1796+
);
1797+
self.detach_exact_keep_slot_prelocked(mount_fs, &mountpoint)
1798+
}
1799+
1800+
fn detach_exact_keep_slot_prelocked(
1801+
&self,
1802+
mount_fs: &Arc<MountFS>,
1803+
mountpoint: &Arc<MountFSInode>,
1804+
) -> Result<Arc<MountFS>, SystemError> {
16051805
if !Arc::ptr_eq(&mountpoint.mount_fs, &self.self_ref()) {
16061806
return Err(SystemError::EINVAL);
16071807
}
1608-
let _gate = mountpoint.dentry.mount_gate.lock();
16091808
let key = mountpoint.dentry.id;
16101809
let mut mountpoints = self.mountpoints.lock();
16111810
let stack = mountpoints.get_mut(&key).ok_or(SystemError::ENOENT)?;
@@ -2279,17 +2478,18 @@ impl MountFS {
22792478
}
22802479

22812480
fn derive_external_pin(&self) -> Result<MountExternalGuard, SystemError> {
2282-
let _topology = MOUNT_LIFECYCLE_LOCK.lock();
22832481
let mut lifecycle = self.lifecycle.lock();
22842482
let component = match lifecycle.state {
2285-
MountLifecycleState::Constructing | MountLifecycleState::Detaching => {
2483+
MountLifecycleState::Constructing => {
22862484
return Err(SystemError::EBUSY);
22872485
}
22882486
MountLifecycleState::Detached if lifecycle.external_pins == 0 => {
22892487
return Err(SystemError::ESTALE);
22902488
}
22912489
MountLifecycleState::DetachedConnected => lifecycle.detached_component.clone(),
2292-
MountLifecycleState::Live | MountLifecycleState::Detached => None,
2490+
MountLifecycleState::Live
2491+
| MountLifecycleState::Detaching
2492+
| MountLifecycleState::Detached => None,
22932493
};
22942494
if component
22952495
.as_ref()
@@ -2710,6 +2910,15 @@ impl MountExternalGuard {
27102910
pub fn derive(&self) -> Result<Self, SystemError> {
27112911
self.mount.derive_external_pin()
27122912
}
2913+
2914+
/// Derive ownership from a guard that is known to remain alive for this
2915+
/// call. Final shutdown and detached-component cleanup both require the
2916+
/// existing owner count to reach zero, so failure indicates an internal
2917+
/// lifecycle-accounting bug rather than a recoverable pathname error.
2918+
pub(crate) fn derive_existing(&self) -> Self {
2919+
self.derive()
2920+
.expect("an existing mount pin must remain derivable")
2921+
}
27132922
}
27142923

27152924
impl Drop for MountExternalGuard {

0 commit comments

Comments
 (0)