Skip to content

Commit 5979107

Browse files
committed
sandlock-core: confine COW metadata ops (stat/statx/utimensat/readlink/copy-up classify)
Signed-off-by: Cong Wang <cwang@multikernel.io>
1 parent 05b7e9f commit 5979107

2 files changed

Lines changed: 140 additions & 103 deletions

File tree

crates/sandlock-core/src/cow/dispatch.rs

Lines changed: 78 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -154,22 +154,35 @@ fn map_cow_upper_path(cow: &SeccompCowBranch, path: &str) -> String {
154154
/// child-controlled path cannot escape the upper/lower tree (issue #112).
155155
/// Picks the anchor root by prefix, then defers all resolution to the kernel
156156
/// via `openat2(RESOLVE_IN_ROOT)`.
157-
fn open_confined(
158-
upper_root: &Path,
159-
workdir_root: &Path,
157+
/// Select the layer root (`upper` first, then `workdir`) that `real_path` lives
158+
/// under and return it with the relative remainder. `Err(EACCES)` if the path
159+
/// is under neither root. This only selects the anchor; the kernel re-resolves
160+
/// the relative path under it with `RESOLVE_IN_ROOT`, so the lexical
161+
/// `strip_prefix` grants no trust.
162+
fn pick_root_rel<'a>(
163+
upper_root: &'a Path,
164+
workdir_root: &'a Path,
160165
real_path: &Path,
161-
flags: i32,
162-
mode: u32,
163-
) -> Result<RawFd, i32> {
166+
) -> Result<(&'a Path, String), i32> {
164167
let (root, rel) = if let Ok(rel) = real_path.strip_prefix(upper_root) {
165168
(upper_root, rel)
166169
} else if let Ok(rel) = real_path.strip_prefix(workdir_root) {
167170
(workdir_root, rel)
168171
} else {
169172
return Err(libc::EACCES);
170173
};
171-
let rel = rel.to_str().ok_or(libc::EINVAL)?;
172-
crate::sys::fs::openat2_in_root(root, rel, flags, mode)
174+
Ok((root, rel.to_str().ok_or(libc::EINVAL)?.to_string()))
175+
}
176+
177+
fn open_confined(
178+
upper_root: &Path,
179+
workdir_root: &Path,
180+
real_path: &Path,
181+
flags: i32,
182+
mode: u32,
183+
) -> Result<RawFd, i32> {
184+
let (root, rel) = pick_root_rel(upper_root, workdir_root, real_path)?;
185+
crate::sys::fs::openat2_in_root(root, &rel, flags, mode)
173186
}
174187

175188
/// Handle openat under workdir: redirect to COW upper/lower.
@@ -702,22 +715,25 @@ pub(crate) async fn handle_cow_utimensat(
702715
None => return NotifAction::Continue,
703716
};
704717

705-
let upper_path = {
718+
let (upper_path, upper_root, workdir_root) = {
706719
let mut st = cow_state.lock().await;
707720
let cow = match st.branch.as_mut() {
708721
Some(c) => c,
709722
None => return NotifAction::Continue,
710723
};
724+
let upper_root = cow.upper_dir().to_path_buf();
725+
let workdir_root = cow.workdir().to_path_buf();
711726
let path = map_cow_upper_path(cow, &path);
712727
if !cow.matches(&path) {
713728
return NotifAction::Continue;
714729
}
715-
match cow.handle_utimensat(&path) {
730+
let p = match cow.handle_utimensat(&path) {
716731
Ok(Some(p)) => p,
717732
Ok(None) => return NotifAction::Continue,
718733
Err(crate::error::BranchError::QuotaExceeded) => return NotifAction::Errno(libc::ENOSPC),
719734
Err(_) => return NotifAction::Continue,
720-
}
735+
};
736+
(p, upper_root, workdir_root)
721737
};
722738

723739
// Read times from child memory (2 x struct timespec = 32 bytes on x86_64)
@@ -736,13 +752,14 @@ pub(crate) async fn handle_cow_utimensat(
736752
None
737753
};
738754

739-
let c_path = match std::ffi::CString::new(upper_path.to_str().unwrap_or("")) {
740-
Ok(c) => c,
741-
Err(_) => return NotifAction::Continue,
755+
let (root, rel) = match pick_root_rel(&upper_root, &workdir_root, &upper_path) {
756+
Ok(v) => v,
757+
Err(e) => return NotifAction::Errno(e),
742758
};
743759
let times_raw = times.as_ref().map(|t| t.as_ptr()).unwrap_or(std::ptr::null());
744-
if unsafe { libc::utimensat(libc::AT_FDCWD, c_path.as_ptr(), times_raw, flags) } < 0 {
745-
return NotifAction::Errno(libc::EIO);
760+
let follow = (flags & libc::AT_SYMLINK_NOFOLLOW) == 0;
761+
if let Err(e) = crate::sys::fs::utimensat_in_root(root, &rel, times_raw, follow) {
762+
return NotifAction::Errno(e);
746763
}
747764
NotifAction::ReturnValue(0)
748765
}
@@ -771,49 +788,52 @@ pub(crate) async fn handle_cow_stat(
771788
None => return NotifAction::Continue,
772789
};
773790

774-
let st = cow_state.lock().await;
775-
let cow = match st.branch.as_ref() {
776-
Some(c) => c,
777-
None => return NotifAction::Continue,
778-
};
779-
780-
let path = map_cow_upper_path(cow, &path);
781-
if !cow.has_changes() || !cow.matches(&path) {
782-
return NotifAction::Continue;
783-
}
784-
785-
let real_path = match cow.handle_stat(&path) {
786-
Some(p) => p,
787-
None => {
788-
return NotifAction::Errno(libc::ENOENT);
791+
let (real_path, upper_root, workdir_root) = {
792+
let st = cow_state.lock().await;
793+
let cow = match st.branch.as_ref() {
794+
Some(c) => c,
795+
None => return NotifAction::Continue,
796+
};
797+
let upper_root = cow.upper_dir().to_path_buf();
798+
let workdir_root = cow.workdir().to_path_buf();
799+
let path = map_cow_upper_path(cow, &path);
800+
if !cow.has_changes() || !cow.matches(&path) {
801+
return NotifAction::Continue;
789802
}
803+
let real = match cow.handle_stat(&path) {
804+
Some(p) => p,
805+
None => return NotifAction::Errno(libc::ENOENT),
806+
};
807+
(real, upper_root, workdir_root)
790808
};
791-
drop(st);
792809

793810
if nr == libc::SYS_faccessat || nr == crate::arch::SYS_FACCESSAT2 {
794-
// For faccessat, just check if the file exists (we already resolved it)
795-
if real_path.exists() || real_path.is_symlink() {
811+
// Existence check, confined: lstat succeeds for any present entry
812+
// (including a dangling symlink), matching the prior semantics.
813+
let (root, rel) = match pick_root_rel(&upper_root, &workdir_root, &real_path) {
814+
Ok(v) => v,
815+
Err(_) => return NotifAction::Errno(libc::ENOENT),
816+
};
817+
if crate::sys::fs::statat_in_root(root, &rel, false).is_ok() {
796818
return NotifAction::ReturnValue(0);
797819
}
798820
return NotifAction::Errno(libc::ENOENT);
799821
}
800822

801-
// newfstatat — stat the resolved path and write the native libc layout
802-
// back to the child. Do not hand-pack struct stat; its layout is
803-
// architecture-specific.
823+
// newfstatat — stat the resolved path (confined to its layer root) and
824+
// write the native libc layout back to the child. Do not hand-pack struct
825+
// stat; its layout is architecture-specific.
804826
let statbuf_addr = notif.data.args[2];
805827
let flags = (notif.data.args[3] & 0xFFFF_FFFF) as i32;
806-
let c_path = match std::ffi::CString::new(real_path.to_str().unwrap_or("")) {
807-
Ok(c) => c,
808-
Err(_) => return NotifAction::Continue,
828+
let follow = (flags & libc::AT_SYMLINK_NOFOLLOW) == 0;
829+
let (root, rel) = match pick_root_rel(&upper_root, &workdir_root, &real_path) {
830+
Ok(v) => v,
831+
Err(e) => return NotifAction::Errno(e),
832+
};
833+
let statbuf = match crate::sys::fs::statat_in_root(root, &rel, follow) {
834+
Ok(s) => s,
835+
Err(e) => return NotifAction::Errno(e),
809836
};
810-
let mut statbuf: libc::stat = unsafe { std::mem::zeroed() };
811-
if unsafe { libc::fstatat(libc::AT_FDCWD, c_path.as_ptr(), &mut statbuf, flags) } < 0 {
812-
let errno = std::io::Error::last_os_error()
813-
.raw_os_error()
814-
.unwrap_or(libc::EIO);
815-
return NotifAction::Errno(errno);
816-
}
817837
let buf = unsafe {
818838
std::slice::from_raw_parts(
819839
&statbuf as *const libc::stat as *const u8,
@@ -859,44 +879,32 @@ pub(crate) async fn handle_cow_statx(
859879
_ => return NotifAction::Continue,
860880
};
861881

862-
let real_path = {
882+
let (real_path, upper_root, workdir_root) = {
863883
let st = cow_state.lock().await;
864884
let cow = match st.branch.as_ref() {
865885
Some(c) => c,
866886
None => return NotifAction::Continue,
867887
};
868-
888+
let upper_root = cow.upper_dir().to_path_buf();
889+
let workdir_root = cow.workdir().to_path_buf();
869890
let path = map_cow_upper_path(cow, &path);
870891
if !cow.has_changes() || !cow.matches(&path) {
871892
return NotifAction::Continue;
872893
}
873-
874-
match cow.handle_stat(&path) {
894+
let real = match cow.handle_stat(&path) {
875895
Some(p) => p,
876896
None => return NotifAction::Errno(libc::ENOENT), // deleted or absent
877-
}
897+
};
898+
(real, upper_root, workdir_root)
878899
};
879900

880-
let c_path = match std::ffi::CString::new(real_path.to_str().unwrap_or("")) {
881-
Ok(c) => c,
882-
Err(_) => return NotifAction::Continue,
901+
let (root, rel) = match pick_root_rel(&upper_root, &workdir_root, &real_path) {
902+
Ok(v) => v,
903+
Err(e) => return NotifAction::Errno(e),
883904
};
884905
let mut stx_buf = vec![0u8; 256]; // sizeof(struct statx)
885-
let ret = unsafe {
886-
libc::syscall(
887-
libc::SYS_statx,
888-
libc::AT_FDCWD,
889-
c_path.as_ptr(),
890-
flags,
891-
mask,
892-
stx_buf.as_mut_ptr(),
893-
)
894-
};
895-
if ret < 0 {
896-
let errno = std::io::Error::last_os_error()
897-
.raw_os_error()
898-
.unwrap_or(libc::EIO);
899-
return NotifAction::Errno(errno);
906+
if let Err(e) = crate::sys::fs::statx_in_root(root, &rel, flags, mask, &mut stx_buf) {
907+
return NotifAction::Errno(e);
900908
}
901909

902910
if write_child_mem(notif_fd, notif.id, notif.pid, statxbuf_addr, &stx_buf).is_err() {

crates/sandlock-core/src/cow/seccomp.rs

Lines changed: 62 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
77
use std::collections::HashSet;
88
use std::fs;
9+
use std::os::unix::ffi::OsStringExt;
10+
use std::os::unix::fs::PermissionsExt;
911
use std::path::{Path, PathBuf};
1012

1113
use crate::error::BranchError;
@@ -207,7 +209,9 @@ impl SeccompCowBranch {
207209
let upper_file = self.upper.join(rel_path);
208210
let lower_file = self.workdir.join(rel_path);
209211

210-
if upper_file.exists() || upper_file.is_symlink() {
212+
// Already materialized in upper? Confined lstat succeeds for any
213+
// existing entry (including a dangling symlink).
214+
if crate::sys::fs::statat_in_root(&self.upper, rel_path, false).is_ok() {
211215
return Ok(CowCopyPlan::Ready(upper_file));
212216
}
213217

@@ -216,46 +220,56 @@ impl SeccompCowBranch {
216220
.map_err(|e| BranchError::Operation(format!("create parent: {}", e)))?;
217221
}
218222

219-
// Symlink — copy immediately (tiny, not worth deferring)
220-
if lower_file.is_symlink() {
223+
// Classify the lower entry confined to the workdir root, so a symlinked
224+
// parent component cannot make us follow out of the tree (issue #112).
225+
// The lstat also yields the size of the entry we will actually copy.
226+
let st = match crate::sys::fs::statat_in_root(&self.workdir, rel_path, false) {
227+
Ok(st) => st,
228+
// Absent or confined-out: treat as a new file created in upper.
229+
Err(libc::ENOENT) => {
230+
self.check_quota(0)?;
231+
return Ok(CowCopyPlan::Ready(upper_file));
232+
}
233+
Err(e) => return Err(BranchError::Operation(format!("stat lower: {}", e))),
234+
};
235+
let kind = st.st_mode & libc::S_IFMT;
236+
237+
// Symlink — copy verbatim (tiny, not worth deferring)
238+
if kind == libc::S_IFLNK {
221239
self.check_quota(256)?;
222-
let target = fs::read_link(&lower_file)
240+
let target = crate::sys::fs::readlink_in_root(&self.workdir, rel_path)
223241
.map_err(|e| BranchError::Operation(format!("readlink: {}", e)))?;
242+
let target = std::path::PathBuf::from(std::ffi::OsString::from_vec(target));
224243
std::os::unix::fs::symlink(&target, &upper_file)
225244
.map_err(|e| BranchError::Operation(format!("symlink: {}", e)))?;
226245
self.disk_used += 256;
227246
return Ok(CowCopyPlan::Ready(upper_file));
228247
}
229248

230249
// Directory — create immediately (no data copy)
231-
if lower_file.is_dir() {
250+
if kind == libc::S_IFDIR {
232251
self.check_quota(4096)?;
233252
fs::create_dir_all(&upper_file)
234253
.map_err(|e| BranchError::Operation(format!("create dir: {}", e)))?;
235-
if let Ok(meta) = lower_file.metadata() {
236-
let _ = fs::set_permissions(&upper_file, meta.permissions());
237-
}
254+
let _ = fs::set_permissions(
255+
&upper_file,
256+
std::fs::Permissions::from_mode(st.st_mode & 0o7777),
257+
);
238258
self.disk_used += 4096;
239259
return Ok(CowCopyPlan::Ready(upper_file));
240260
}
241261

242-
// Regular file — defer the potentially expensive copy
243-
if lower_file.exists() {
244-
let meta = lower_file.metadata()
245-
.map_err(|e| BranchError::Operation(format!("metadata: {}", e)))?;
246-
let file_size = meta.len();
247-
self.check_quota(file_size)?;
248-
self.disk_used += file_size;
249-
return Ok(CowCopyPlan::NeedsCopy {
250-
upper: upper_file,
251-
lower: lower_file,
252-
file_size,
253-
});
254-
}
255-
256-
// New file (not in lower layer)
257-
self.check_quota(0)?;
258-
Ok(CowCopyPlan::Ready(upper_file))
262+
// Regular file — defer the potentially expensive copy. Size comes from
263+
// the confined lstat, so the quota reservation matches the file
264+
// execute_copy will actually read.
265+
let file_size = st.st_size as u64;
266+
self.check_quota(file_size)?;
267+
self.disk_used += file_size;
268+
Ok(CowCopyPlan::NeedsCopy {
269+
upper: upper_file,
270+
lower: lower_file,
271+
file_size,
272+
})
259273
}
260274

261275
/// Execute a file copy synchronously. Used by `ensure_cow_copy` and the
@@ -704,15 +718,14 @@ impl SeccompCowBranch {
704718
if self.is_deleted(&rel) {
705719
return None;
706720
}
707-
let upper = self.upper.join(&rel);
708-
let lower = self.workdir.join(&rel);
709-
if upper.is_symlink() {
710-
fs::read_link(&upper).ok().map(|p| p.to_string_lossy().into_owned())
711-
} else if lower.is_symlink() {
712-
fs::read_link(&lower).ok().map(|p| p.to_string_lossy().into_owned())
713-
} else {
714-
None
721+
// Read the link confined to each layer root so a symlinked parent
722+
// component cannot escape the tree (issue #112).
723+
for root in [&self.upper, &self.workdir] {
724+
if let Ok(target) = crate::sys::fs::readlink_in_root(root, &rel) {
725+
return Some(String::from_utf8_lossy(&target).into_owned());
726+
}
715727
}
728+
None
716729
}
717730

718731
/// List all filesystem changes in the COW layer.
@@ -1554,4 +1567,20 @@ mod tests {
15541567
let meta = std::fs::symlink_metadata(&committed).unwrap();
15551568
assert!(meta.file_type().is_symlink());
15561569
}
1570+
1571+
#[test]
1572+
fn cow_copy_preserves_in_tree_symlink() {
1573+
// The confined-stat classification in prepare_copy must still treat an
1574+
// in-tree symlink as a symlink and copy it verbatim into upper.
1575+
let (workdir, storage) = setup_workdir();
1576+
std::os::unix::fs::symlink("existing.txt", workdir.path().join("link")).unwrap();
1577+
let mut branch =
1578+
SeccompCowBranch::create(workdir.path(), Some(storage.path()), 0).unwrap();
1579+
let upper = branch.ensure_cow_copy("link").unwrap();
1580+
assert!(upper.is_symlink(), "in-tree symlink was not preserved");
1581+
assert_eq!(
1582+
std::fs::read_link(&upper).unwrap(),
1583+
std::path::Path::new("existing.txt")
1584+
);
1585+
}
15571586
}

0 commit comments

Comments
 (0)