From 889e861e9d1043a3469eab76c733b64e2bd4ec14 Mon Sep 17 00:00:00 2001 From: Jonathan Krebs Date: Mon, 22 Apr 2024 23:31:55 +0200 Subject: [PATCH 1/4] mount tests: take FORK_MTX when forking or unmounting unmounting can fail if a child process inherited a file a file descriptor we opened in temporary mount point Should fix #2369 --- test/mount/test_mount.rs | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/test/mount/test_mount.rs b/test/mount/test_mount.rs index a4f0903dba..65004465d5 100644 --- a/test/mount/test_mount.rs +++ b/test/mount/test_mount.rs @@ -53,6 +53,8 @@ fn test_mount_tmpfs_without_flags_allows_rwx() { .unwrap_or_else(|e| panic!("read failed: {e}")); assert_eq!(buf, SCRIPT_CONTENTS); + // while forking and unmounting prevent other child processes + let _m = FORK_MTX.lock(); // Verify execute. assert_eq!( EXPECTED_STATUS, @@ -89,6 +91,8 @@ fn test_mount_rdonly_disallows_write() { .unwrap() ); + // wait for child processes to prevent EBUSY + let _m = FORK_MTX.lock(); umount(tempdir.path()).unwrap_or_else(|e| panic!("umount failed: {e}")); } @@ -129,6 +133,8 @@ fn test_mount_noexec_disallows_exec() { &test_path ); + // while forking and unmounting prevent other child processes + let _m = FORK_MTX.lock(); // EACCES: Permission denied assert_eq!( EACCES, @@ -168,6 +174,8 @@ fn test_mount_bind() { .and_then(|mut f| f.write(SCRIPT_CONTENTS)) .unwrap_or_else(|e| panic!("write failed: {e}")); + // wait for child processes to prevent EBUSY + let _m = FORK_MTX.lock(); umount(mount_point.path()) .unwrap_or_else(|e| panic!("umount failed: {e}")); } From f7c327c2d017658a017e6f1f4cfd0647e48b4ef6 Mon Sep 17 00:00:00 2001 From: Jonathan Krebs Date: Mon, 22 Apr 2024 23:59:00 +0200 Subject: [PATCH 2/4] tests: clarify the meaning of the FORK_MTX --- test/test.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/test.rs b/test/test.rs index 53a6af4b6c..f2e456b743 100644 --- a/test/test.rs +++ b/test/test.rs @@ -58,7 +58,8 @@ fn read_exact(f: Fd, buf: &mut [u8]) { } /// Any test that creates child processes must grab this mutex, regardless -/// of what it does with those children. +/// of what it does with those children. It must hold the mutex until the +/// child processes are waited upon. pub static FORK_MTX: Mutex<()> = Mutex::new(()); /// Any test that changes the process's current working directory must grab /// the RwLock exclusively. Any process that cares about the current From 93fb6e6b6e3c0256758667dea1afff10c442e148 Mon Sep 17 00:00:00 2001 From: TheJonny Date: Wed, 24 Apr 2024 02:21:55 +0200 Subject: [PATCH 3/4] tests: clarify the meaning of FORK_MTX Co-authored-by: SteveLauC --- test/test.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/test.rs b/test/test.rs index f2e456b743..4e0ffc8f9a 100644 --- a/test/test.rs +++ b/test/test.rs @@ -57,7 +57,7 @@ fn read_exact(f: Fd, buf: &mut [u8]) { } } -/// Any test that creates child processes must grab this mutex, regardless +/// Any test that creates child processes or can be affected by child processes must grab this mutex, regardless /// of what it does with those children. It must hold the mutex until the /// child processes are waited upon. pub static FORK_MTX: Mutex<()> = Mutex::new(()); From 56eaed7ce61a727e30c1155c01e7caa058ab3b17 Mon Sep 17 00:00:00 2001 From: Jonathan Krebs Date: Wed, 24 Apr 2024 03:18:25 +0200 Subject: [PATCH 4/4] here no one accesses the file system to be unmounted, so no fork lock needed --- test/mount/test_mount.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/mount/test_mount.rs b/test/mount/test_mount.rs index 65004465d5..9cb7741796 100644 --- a/test/mount/test_mount.rs +++ b/test/mount/test_mount.rs @@ -91,8 +91,6 @@ fn test_mount_rdonly_disallows_write() { .unwrap() ); - // wait for child processes to prevent EBUSY - let _m = FORK_MTX.lock(); umount(tempdir.path()).unwrap_or_else(|e| panic!("umount failed: {e}")); }