From 612be884665d1029e571f3d9b1e8fc30b8714b66 Mon Sep 17 00:00:00 2001
From: xujihui1985 <xujihui1985@gmail.com>
Date: Mon, 14 Oct 2024 21:32:37 +0800
Subject: [PATCH] fix: add comments to no-pivot related code

---
 crates/libcontainer/src/process/container_init_process.rs | 8 ++++++++
 tests/contest/runtimetest/src/tests.rs                    | 7 +++++++
 2 files changed, 15 insertions(+)

diff --git a/crates/libcontainer/src/process/container_init_process.rs b/crates/libcontainer/src/process/container_init_process.rs
index 93d5f9e853..d6fb9a7953 100644
--- a/crates/libcontainer/src/process/container_init_process.rs
+++ b/crates/libcontainer/src/process/container_init_process.rs
@@ -270,6 +270,9 @@ fn reopen_dev_null() -> Result<()> {
     Ok(())
 }
 
+// umount or hide the target path. If the target path is mounted
+// try to unmount it first if the unmount operation fails with EINVAL
+// then mount a tmpfs with size 0k to hide the target path.
 fn unmount_or_hide(syscall: &dyn Syscall, target: impl AsRef<Path>) -> Result<()> {
     let target_path = target.as_ref();
     match syscall.umount2(target_path, MntFlags::MNT_DETACH) {
@@ -289,6 +292,11 @@ fn unmount_or_hide(syscall: &dyn Syscall, target: impl AsRef<Path>) -> Result<()
 
 fn move_root(syscall: &dyn Syscall, rootfs: &Path) -> Result<()> {
     unistd::chdir(rootfs).map_err(InitProcessError::NixOther)?;
+    // umount /sys and /proc if they are mounted, the purpose is to 
+    // unmount or hide the /sys and /proc filesystems before the process changes its 
+    // root to the new rootfs. thus ensure that the /sys and /proc filesystems are not
+    // accessible in the new rootfs. the logic is borrowed from crun
+    // https://github.com/containers/crun/blob/53cd1c1c697d7351d0cad23708d29bf4a7980a3a/src/libcrun/linux.c#L2780
     unmount_or_hide(syscall, "/sys")?;
     unmount_or_hide(syscall, "/proc")?;
     syscall
diff --git a/tests/contest/runtimetest/src/tests.rs b/tests/contest/runtimetest/src/tests.rs
index 0e0f0080c2..dec34dee35 100644
--- a/tests/contest/runtimetest/src/tests.rs
+++ b/tests/contest/runtimetest/src/tests.rs
@@ -546,7 +546,10 @@ pub fn test_io_priority_class(spec: &Spec, io_priority_class: IOPriorityClass) {
     }
 }
 
+// the validate_rootfs function is used to validate the rootfs of the container is
+// as expected. This function is used in the no_pivot test to validate the rootfs
 pub fn validate_rootfs() {
+    // list the first level directories in the rootfs
     let mut entries = fs::read_dir("/")
         .unwrap()
         .filter_map(|entry| {
@@ -561,13 +564,17 @@ pub fn validate_rootfs() {
             })
         })
         .collect::<Vec<String>>();
+    // sort the entries to make the test deterministic
     entries.sort();
 
+    // this is the list of directories that we expect to find in the rootfs
     let mut expected = vec![
         "bin", "dev", "etc", "home", "proc", "root", "sys", "tmp", "usr", "var",
     ];
+    // sort the expected entries to make the test deterministic
     expected.sort();
 
+    // compare the expected entries with the actual entries
     if entries != expected {
         eprintln!("error due to rootfs want {expected:?}, got {entries:?}");
     }