From bd692b04310bf10ce991238da63f3326c4520ce3 Mon Sep 17 00:00:00 2001 From: lind Date: Thu, 27 Jun 2024 17:56:19 +0000 Subject: [PATCH 1/8] finished comments and tests for getcw --- src/safeposix/syscalls/fs_calls.rs | 77 ++++++++++++++++++++---- src/tests/fs_tests.rs | 94 +++++++++++++++++++++++------- 2 files changed, 139 insertions(+), 32 deletions(-) diff --git a/src/safeposix/syscalls/fs_calls.rs b/src/safeposix/syscalls/fs_calls.rs index 53723c05c..d6d7f44a9 100644 --- a/src/safeposix/syscalls/fs_calls.rs +++ b/src/safeposix/syscalls/fs_calls.rs @@ -3461,22 +3461,77 @@ impl Cage { } } - //------------------------------------GETCWD SYSCALL------------------------------------ + /// ### Description + /// + /// The `getcwd_syscall()` function places an absolute pathname of the + /// current working directory in the string pointed to by buf. + /// + /// ### Arguments + /// + /// The `getcwd_syscall()` accepts two arguments: + /// * `buf` - a pointer to the string into which the current working + /// directory is stored + /// * `bufsize` - the length of the string `buf` + /// + /// ### Returns + /// + /// The standard requires returning the pointer to the string that + /// stores the current working directory. In the current implementation, + /// 0 is returned on success, while returning the pointer to the string is + /// handled inside glibc. + /// In case of a failure, an error is returned, and `errno` is set depending + /// on the error, e.g. EINVAL, ERANGE, etc. + /// + /// ### Errors + /// + /// * `EINVAL` - the bufsize argument is zero and buf is not a NULL pointer. + /// * `ERANGE` - the bufsize argument is less than the length of the absolute + /// pathname of the working directory, including the terminating null byte. + /// * `EFAULT` - buf points to a bad address. + /// Other errors, like `EACCES`, `ENOMEM`, etc. are not supported. + /// + /// ### Panics + /// + /// There are no cases where this function panics. + /// + /// To learn more about the syscall and possible error values, see + /// [getcwd(3)](https://man7.org/linux/man-pages/man3/getcwd.3.html) - pub fn getcwd_syscall(&self, buf: *mut u8, bufsize: u32) -> i32 { - let mut bytes: Vec = self.cwd.read().to_str().unwrap().as_bytes().to_vec(); - bytes.push(0u8); //Adding a null terminator to the end of the string - let length = bytes.len(); - if (bufsize as usize) < length { - return syscall_error(Errno::ERANGE, "getcwd", "the length (in bytes) of the absolute pathname of the current working directory exceeds the given size"); + pub fn getcwd_syscall(&self, buf: *mut u8, bufsize: u32) -> i32 { + //The first two conditions help to error-out quickly if either + //the pointer to the string is a null pointer or the specified + //size of the string is 0. + if !buf.is_null() { + if (bufsize as usize) == 0 { + return syscall_error(Errno::EINVAL, "getcwd", "size of the specified buffer is 0"); + } else { + //Cages store their current working directory as path buffers. + //To use the obtained directory as a string, a null terminator needs + //to be added to the path. + let mut bytes: Vec = self.cwd.read().to_str().unwrap().as_bytes().to_vec(); + bytes.push(0u8); //Adding a null terminator to the end of the string + let length = bytes.len(); + //The bufsize argument should be at least the length of the absolute + //pathname of the working directory, including the terminating null byte. + if (bufsize as usize) < length { + return syscall_error(Errno::ERANGE, "getcwd", "the length (in bytes) of the absolute pathname of the current working directory exceeds the given size"); + } + //It is expected that only the first `bufsize` bytes of the `buf` string + //will be written into. The `fill()` function ensures this by taking + //a mutable slice of length `bufsize` to the string pointed to by `buf` + //and inserting the obtained current working directory into that slice, + //thus prohibiting writing into the remaining bytes of the string. + interface::fill(buf, length, &bytes); + //returning 0 on success + 0 + } + } else { + return syscall_error(Errno::EFAULT, "getcwd", "an invalid (null) pointer to the buffer is provided"); } - - interface::fill(buf, length, &bytes); - - 0 //getcwd has succeeded!; } + //------------------SHMHELPERS---------------------- pub fn rev_shm_find_index_by_addr(rev_shm: &Vec<(u32, i32)>, shmaddr: u32) -> Option { diff --git a/src/tests/fs_tests.rs b/src/tests/fs_tests.rs index 7a06ddf23..cbec44471 100644 --- a/src/tests/fs_tests.rs +++ b/src/tests/fs_tests.rs @@ -1352,39 +1352,91 @@ pub mod fs_tests { } #[test] - pub fn ut_lind_fs_dir_chdir_getcwd() { - //acquiring a lock on TESTMUTEX prevents other tests from running concurrently, and also performs clean env setup - let _thelock = setup::lock_and_init(); - + pub fn ut_lind_fs_getcwd_valid_args() { + //Acquiring a lock on TESTMUTEX prevents other tests from running + //concurrently, and also performs clean env setup + let _thelock = setup::lock_and_init(); let cage = interface::cagetable_getref(1); - let needed = "/subdir1\0".as_bytes().to_vec().len(); - - let needed_u32: u32 = needed as u32; - let mut buf = vec![0u8; needed]; + //Checking if retrieving the current working directory + //`/newcwd1` by passing a string long enough to store + //the directory succeeds + //`newcwdsize` is the size of the string needed to store the new + //current woring directory `/newcwd1` + let newcwdsize = "/newcwd1\0".as_bytes().to_vec().len(); + let newcwdsize_u32: u32 = newcwdsize as u32; + //Creating the new current working directory and checking if it exits + cage.mkdir_syscall("/newcwd1", S_IRWXA); + assert_eq!(cage.access_syscall("newcwd1", F_OK), 0); + //Creating a string large enough to store the new current + //working directory and filling it with 0's + let mut buf = vec![0u8; newcwdsize]; let bufptr: *mut u8 = &mut buf[0]; + //First, we change to the root directory and call `getcwd_syscall()` + //to check if the root current working directory is successfully + //retrieved assert_eq!(cage.chdir_syscall("/"), 0); - assert_eq!(cage.getcwd_syscall(bufptr, 0), -(Errno::ERANGE as i32)); - assert_eq!(cage.getcwd_syscall(bufptr, 1), -(Errno::ERANGE as i32)); assert_eq!(cage.getcwd_syscall(bufptr, 2), 0); assert_eq!(std::str::from_utf8(&buf).unwrap(), "/\0\0\0\0\0\0\0\0"); - cage.mkdir_syscall("/subdir1", S_IRWXA); - assert_eq!(cage.access_syscall("subdir1", F_OK), 0); - assert_eq!(cage.chdir_syscall("subdir1"), 0); - - assert_eq!(cage.getcwd_syscall(bufptr, 0), -(Errno::ERANGE as i32)); - assert_eq!( - cage.getcwd_syscall(bufptr, needed_u32 - 1), - -(Errno::ERANGE as i32) - ); - assert_eq!(cage.getcwd_syscall(bufptr, needed_u32), 0); - assert_eq!(std::str::from_utf8(&buf).unwrap(), "/subdir1\0"); + //Now, we change to the newly created directory and check if it is + //successfully retrieved as a new current working directory + assert_eq!(cage.chdir_syscall("/newcwd1"), 0); + assert_eq!(cage.getcwd_syscall(bufptr, newcwdsize_u32), 0); + assert_eq!(std::str::from_utf8(&buf).unwrap(), "/newcwd1\0"); + + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); + lindrustfinalize(); + } + + + #[test] + pub fn ut_lind_fs_getcwd_invalid_args() { + //Acquiring a lock on TESTMUTEX prevents other tests from running + //concurrently, and also performs clean env setup + let _thelock = setup::lock_and_init(); + let cage = interface::cagetable_getref(1); + //`/newcwd2` is a valid directory that we will create to check + //if calling `getcwd_syscall()` with invalid arguments + //correctly returns the required errors. + //`newcwdsize` is the size of the string needed to store the new + //current woring directory `/newcwd1` + let newcwdsize = "/newcwd2\0".as_bytes().to_vec().len(); + let newcwdsize_u32: u32 = newcwdsize as u32; + //Creating the new current working directory, checking if it exits + //and changing to it + cage.mkdir_syscall("/newcwd2", S_IRWXA); + assert_eq!(cage.access_syscall("newcwd2", F_OK), 0); + assert_eq!(cage.chdir_syscall("newcwd2"), 0); + + //Checking if passing a null pointer and the correct string size + //to `getcwd_syscall()`correctly results in `Buf points to + //a bad address` error + let null_ptr: *mut u8 = std::ptr::null_mut(); + assert_eq!(cage.getcwd_syscall(null_ptr, newcwdsize_u32), -(Errno::EFAULT as i32)); + + //Checking if passing a valid string pointer and a size of 0 to + //`getcwd_syscall()` correctly results in `The size argument is zero and + //buf is not a null pointer` error + //Creating a string large enough to store the new current + //working directory and filling it with 0's + let mut buf = vec![0u8; newcwdsize]; + let bufptr: *mut u8 = &mut buf[0]; + assert_eq!(cage.getcwd_syscall(bufptr, 0), -(Errno::EINVAL as i32)); + + //Checking if passing a valid string pointer and a non-zero size smaller + //than the size of the current working direcotry plus the terminating null + //character to `getcwd_syscall()` correctly results in `The bufsize argument + //is less than the length of the absolute pathname of the working directory, + //including the terminating null byte` error + assert_eq!(cage.getcwd_syscall(bufptr, newcwdsize_u32 - 1), -(Errno::ERANGE as i32)); + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); lindrustfinalize(); } + #[test] pub fn ut_lind_fs_exec_cloexec() { From fb06122299a22749f90ed7c32685f05ea6dc59ce Mon Sep 17 00:00:00 2001 From: lind Date: Thu, 27 Jun 2024 17:57:55 +0000 Subject: [PATCH 2/8] removed extra whitespace --- src/safeposix/syscalls/fs_calls.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/safeposix/syscalls/fs_calls.rs b/src/safeposix/syscalls/fs_calls.rs index d6d7f44a9..1eeecf759 100644 --- a/src/safeposix/syscalls/fs_calls.rs +++ b/src/safeposix/syscalls/fs_calls.rs @@ -3531,7 +3531,6 @@ impl Cage { } } - //------------------SHMHELPERS---------------------- pub fn rev_shm_find_index_by_addr(rev_shm: &Vec<(u32, i32)>, shmaddr: u32) -> Option { From 8d5c9a08b7dc936fd3dd4dc6885dc50be06ad71e Mon Sep 17 00:00:00 2001 From: lind Date: Thu, 27 Jun 2024 17:58:37 +0000 Subject: [PATCH 3/8] removed extra whitespace --- src/tests/fs_tests.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tests/fs_tests.rs b/src/tests/fs_tests.rs index cbec44471..15bdd14ab 100644 --- a/src/tests/fs_tests.rs +++ b/src/tests/fs_tests.rs @@ -1435,8 +1435,7 @@ pub mod fs_tests { assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); lindrustfinalize(); - } - + } #[test] pub fn ut_lind_fs_exec_cloexec() { From a119c397dedd26c3a026f7f5bdfd441756ad639d Mon Sep 17 00:00:00 2001 From: lind Date: Thu, 27 Jun 2024 18:00:07 +0000 Subject: [PATCH 4/8] removed extra whitespace --- src/tests/fs_tests.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/tests/fs_tests.rs b/src/tests/fs_tests.rs index 15bdd14ab..f3d5df5b2 100644 --- a/src/tests/fs_tests.rs +++ b/src/tests/fs_tests.rs @@ -1388,8 +1388,7 @@ pub mod fs_tests { assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); lindrustfinalize(); - } - + } #[test] pub fn ut_lind_fs_getcwd_invalid_args() { From f2bb0773508d6f7bc1e84af3ae748361b79369ef Mon Sep 17 00:00:00 2001 From: lind Date: Thu, 27 Jun 2024 18:14:03 +0000 Subject: [PATCH 5/8] formatted with cargofmt --- src/safeposix/syscalls/fs_calls.rs | 17 +++++++------ src/tests/fs_tests.rs | 38 +++++++++++++++++------------- 2 files changed, 31 insertions(+), 24 deletions(-) diff --git a/src/safeposix/syscalls/fs_calls.rs b/src/safeposix/syscalls/fs_calls.rs index 1eeecf759..a2a813204 100644 --- a/src/safeposix/syscalls/fs_calls.rs +++ b/src/safeposix/syscalls/fs_calls.rs @@ -3497,15 +3497,14 @@ impl Cage { /// To learn more about the syscall and possible error values, see /// [getcwd(3)](https://man7.org/linux/man-pages/man3/getcwd.3.html) - pub fn getcwd_syscall(&self, buf: *mut u8, bufsize: u32) -> i32 { - //The first two conditions help to error-out quickly if either - //the pointer to the string is a null pointer or the specified - //size of the string is 0. + //The first two conditions help to error-out quickly if either + //the pointer to the string is a null pointer or the specified + //size of the string is 0. if !buf.is_null() { if (bufsize as usize) == 0 { return syscall_error(Errno::EINVAL, "getcwd", "size of the specified buffer is 0"); - } else { + } else { //Cages store their current working directory as path buffers. //To use the obtained directory as a string, a null terminator needs //to be added to the path. @@ -3526,8 +3525,12 @@ impl Cage { //returning 0 on success 0 } - } else { - return syscall_error(Errno::EFAULT, "getcwd", "an invalid (null) pointer to the buffer is provided"); + } else { + return syscall_error( + Errno::EFAULT, + "getcwd", + "an invalid (null) pointer to the buffer is provided", + ); } } diff --git a/src/tests/fs_tests.rs b/src/tests/fs_tests.rs index f3d5df5b2..e32a15f5b 100644 --- a/src/tests/fs_tests.rs +++ b/src/tests/fs_tests.rs @@ -12,7 +12,6 @@ pub mod fs_tests { #[test] pub fn ut_lind_fs_simple() { - //acquiring a lock on TESTMUTEX prevents other tests from running concurrently, and also performs clean env setup let _thelock = setup::lock_and_init(); @@ -142,9 +141,9 @@ pub mod fs_tests { pub fn ut_lind_fs_broken_close() { //acquiring a lock on TESTMUTEX prevents other tests from running concurrently, and also performs clean env setup let _thelock = setup::lock_and_init(); - + //testing a muck up with the inode table where the regular close does not work as intended - + let cage = interface::cagetable_getref(1); //write should work @@ -564,7 +563,7 @@ pub mod fs_tests { } #[test] - pub fn ut_lind_fs_fcntl_invalid_args(){ + pub fn ut_lind_fs_fcntl_invalid_args() { //acquiring a lock on TESTMUTEX prevents other tests from running concurrently, and also performs clean env setup let _thelock = setup::lock_and_init(); @@ -595,7 +594,7 @@ pub mod fs_tests { } #[test] - pub fn ut_lind_fs_fcntl_dup(){ + pub fn ut_lind_fs_fcntl_dup() { //acquiring a lock on TESTMUTEX prevents other tests from running concurrently, and also performs clean env setup let _thelock = setup::lock_and_init(); @@ -670,7 +669,7 @@ pub mod fs_tests { pub fn ut_lind_fs_ioctl_invalid_args() { //acquiring a lock on TESTMUTEX prevents other tests from running concurrently, and also performs clean env setup let _thelock = setup::lock_and_init(); - + let cage = interface::cagetable_getref(1); //setting up two integer values (a zero value to test clearing nonblocking I/O behavior on @@ -725,7 +724,7 @@ pub mod fs_tests { assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); lindrustfinalize(); - } + } #[test] pub fn ut_lind_fs_fdflags() { @@ -1355,7 +1354,7 @@ pub mod fs_tests { pub fn ut_lind_fs_getcwd_valid_args() { //Acquiring a lock on TESTMUTEX prevents other tests from running //concurrently, and also performs clean env setup - let _thelock = setup::lock_and_init(); + let _thelock = setup::lock_and_init(); let cage = interface::cagetable_getref(1); //Checking if retrieving the current working directory @@ -1385,16 +1384,16 @@ pub mod fs_tests { assert_eq!(cage.chdir_syscall("/newcwd1"), 0); assert_eq!(cage.getcwd_syscall(bufptr, newcwdsize_u32), 0); assert_eq!(std::str::from_utf8(&buf).unwrap(), "/newcwd1\0"); - + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); lindrustfinalize(); - } - + } + #[test] pub fn ut_lind_fs_getcwd_invalid_args() { //Acquiring a lock on TESTMUTEX prevents other tests from running //concurrently, and also performs clean env setup - let _thelock = setup::lock_and_init(); + let _thelock = setup::lock_and_init(); let cage = interface::cagetable_getref(1); //`/newcwd2` is a valid directory that we will create to check @@ -1414,7 +1413,10 @@ pub mod fs_tests { //to `getcwd_syscall()`correctly results in `Buf points to //a bad address` error let null_ptr: *mut u8 = std::ptr::null_mut(); - assert_eq!(cage.getcwd_syscall(null_ptr, newcwdsize_u32), -(Errno::EFAULT as i32)); + assert_eq!( + cage.getcwd_syscall(null_ptr, newcwdsize_u32), + -(Errno::EFAULT as i32) + ); //Checking if passing a valid string pointer and a size of 0 to //`getcwd_syscall()` correctly results in `The size argument is zero and @@ -1430,11 +1432,14 @@ pub mod fs_tests { //character to `getcwd_syscall()` correctly results in `The bufsize argument //is less than the length of the absolute pathname of the working directory, //including the terminating null byte` error - assert_eq!(cage.getcwd_syscall(bufptr, newcwdsize_u32 - 1), -(Errno::ERANGE as i32)); - + assert_eq!( + cage.getcwd_syscall(bufptr, newcwdsize_u32 - 1), + -(Errno::ERANGE as i32) + ); + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); lindrustfinalize(); - } + } #[test] pub fn ut_lind_fs_exec_cloexec() { @@ -1514,7 +1519,6 @@ pub mod fs_tests { //acquiring a lock on TESTMUTEX prevents other tests from running concurrently, and also performs clean env setup let _thelock = setup::lock_and_init(); - let cage1 = interface::cagetable_getref(1); let pid1 = cage1.getpid_syscall(); From 51d4c1a4d7320139b167f71be7adfdf3da1386c2 Mon Sep 17 00:00:00 2001 From: lind Date: Mon, 1 Jul 2024 15:50:15 +0000 Subject: [PATCH 6/8] fixed a typo --- src/tests/fs_tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/fs_tests.rs b/src/tests/fs_tests.rs index 8f271b96d..c909df65c 100644 --- a/src/tests/fs_tests.rs +++ b/src/tests/fs_tests.rs @@ -1487,7 +1487,7 @@ pub mod fs_tests { assert_eq!(cage.getcwd_syscall(bufptr, 0), -(Errno::EINVAL as i32)); //Checking if passing a valid string pointer and a non-zero size smaller - //than the size of the current working direcotry plus the terminating null + //than the size of the current working directory plus the terminating null //character to `getcwd_syscall()` correctly results in `The bufsize argument //is less than the length of the absolute pathname of the working directory, //including the terminating null byte` error From 940679b7a95944f28386c3b74b5fb45d6ecdd6d8 Mon Sep 17 00:00:00 2001 From: lind Date: Mon, 1 Jul 2024 15:51:09 +0000 Subject: [PATCH 7/8] formatted --- benches/fs_open_close.rs | 2 +- benches/fs_read_write.rs | 2 +- benches/fs_read_write_seek.rs | 2 +- benches/gen_getid.rs | 2 +- src/safeposix/syscalls/fs_calls.rs | 472 ++++++++++++++++------------- src/tests/fs_tests.rs | 43 +-- 6 files changed, 295 insertions(+), 228 deletions(-) diff --git a/benches/fs_open_close.rs b/benches/fs_open_close.rs index b0d2ec73d..f4cf3dba2 100644 --- a/benches/fs_open_close.rs +++ b/benches/fs_open_close.rs @@ -1,5 +1,5 @@ /* Benchmarks for the microvisor implementation. In general, I'm not doing - * results checking / assertations to avoid adding bias to the results. */ + * results checking / assertations to avoid adding bias to the results. */ // I hate allowing this, but this is apparently a known issue for a lot of // code with CStrings. https://github.com/rust-lang/rust/issues/78691 diff --git a/benches/fs_read_write.rs b/benches/fs_read_write.rs index dcf73b911..32c0f0064 100644 --- a/benches/fs_read_write.rs +++ b/benches/fs_read_write.rs @@ -1,5 +1,5 @@ /* Benchmarks for the microvisor implementation. In general, I'm not doing - * results checking / assertations to avoid adding bias to the results. */ + * results checking / assertations to avoid adding bias to the results. */ use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion}; diff --git a/benches/fs_read_write_seek.rs b/benches/fs_read_write_seek.rs index bc7eec58c..a48d3c99e 100644 --- a/benches/fs_read_write_seek.rs +++ b/benches/fs_read_write_seek.rs @@ -1,5 +1,5 @@ /* Benchmarks for the microvisor implementation. In general, I'm not doing - * results checking / assertations to avoid adding bias to the results. */ + * results checking / assertations to avoid adding bias to the results. */ use criterion::{criterion_group, criterion_main, BenchmarkId, Criterion}; diff --git a/benches/gen_getid.rs b/benches/gen_getid.rs index 688757a29..29c973c85 100644 --- a/benches/gen_getid.rs +++ b/benches/gen_getid.rs @@ -1,5 +1,5 @@ /* Benchmarks for the microvisor implementation. In general, I'm not doing - * results checking / assertations to avoid adding bias to the results. */ + * results checking / assertations to avoid adding bias to the results. */ use criterion::{criterion_group, criterion_main, Criterion}; diff --git a/src/safeposix/syscalls/fs_calls.rs b/src/safeposix/syscalls/fs_calls.rs index b09c4dd16..cd402b0b4 100644 --- a/src/safeposix/syscalls/fs_calls.rs +++ b/src/safeposix/syscalls/fs_calls.rs @@ -3623,7 +3623,8 @@ impl Cage { /// ### Errors /// /// * `EINVAL` - the bufsize argument is zero and buf is not a NULL pointer. - /// * `ERANGE` - the bufsize argument is less than the length of the absolute + /// * `ERANGE` - the bufsize argument is less than the length of the + /// absolute /// pathname of the working directory, including the terminating null byte. /// * `EFAULT` - buf points to a bad address. /// Other errors, like `EACCES`, `ENOMEM`, etc. are not supported. @@ -4296,44 +4297,49 @@ impl Cage { } //##------------------SEMAPHORE SYSCALLS------------------ -/* - * Initialize semaphore object SEM to value - * pshared used to indicate whether the semaphore is shared in threads (when equals to 0) - * or shared between processes (when nonzero) - */ -/// ## `sem_init_syscall` -/// -/// ### Description -/// This function initializes a semaphore object, setting its initial value and -///specifying whether it's shared between threads or processes. -/// 1. Boundary Check: The function first checks if the initial value is -/// within the allowed range. -/// 2. Check for Existing Semaphore: The function then checks if a semaphore -/// with the given handle already exists. -/// 3. Initialize New Semaphore: If the semaphore does not exist, the function -/// creates a new semaphore object and inserts it into the semaphore table. -/// 4. Add to Shared Memory Attachments (if shared): If the semaphore is shared -/// between processes, -/// the function adds it to the shared memory attachments of other processes -/// that have already attached to the shared memory segment. -/// 5. The function ensures thread safety by using a unique semaphore handle and -/// checking for existing entries in the semaphore table before attempting to create a new one. -/// The code also avoids inserting a semaphore into the same cage twice during the shared -/// memory attachment process by excluding the initial cage from the iteration loop. -///[sem_init](https://man7.org/linux/man-pages/man3/sem_init.3.html) -/// ### Function Arguments -/// * `sem_handle`: A unique identifier for the semaphore. -/// * `pshared`: Indicates whether the semaphore is shared between -/// threads (0) or processes (non-zero). -/// * `value`: The initial value of the semaphore. -/// -/// ### Returns -/// * 0 on success. -/// ### Errors -/// * `EBADF (9)`: If the semaphore handle is invalid or the semaphore is already initialized. -/// * `EINVAL (22)`: If the initial value exceeds the maximum allowable value -/// * 'ENOSYS(38)' : Currently not supported -/// for a semaphore (SEM_VALUE_MAX). + /* + * Initialize semaphore object SEM to value + * pshared used to indicate whether the semaphore is shared in threads (when + * equals to 0) or shared between processes (when nonzero) + */ + /// ## `sem_init_syscall` + /// + /// ### Description + /// This function initializes a semaphore object, setting its initial value + /// and specifying whether it's shared between threads or processes. + /// 1. Boundary Check: The function first checks if the initial value is + /// within the allowed range. + /// 2. Check for Existing Semaphore: The function then checks if a semaphore + /// with the given handle already exists. + /// 3. Initialize New Semaphore: If the semaphore does not exist, the + /// function + /// creates a new semaphore object and inserts it into the semaphore table. + /// 4. Add to Shared Memory Attachments (if shared): If the semaphore is + /// shared + /// between processes, + /// the function adds it to the shared memory attachments of other processes + /// that have already attached to the shared memory segment. + /// 5. The function ensures thread safety by using a unique semaphore handle + /// and + /// checking for existing entries in the semaphore table before attempting + /// to create a new one. The code also avoids inserting a semaphore into + /// the same cage twice during the shared memory attachment process by + /// excluding the initial cage from the iteration loop. [sem_init](https://man7.org/linux/man-pages/man3/sem_init.3.html) + /// ### Function Arguments + /// * `sem_handle`: A unique identifier for the semaphore. + /// * `pshared`: Indicates whether the semaphore is shared between + /// threads (0) or processes (non-zero). + /// * `value`: The initial value of the semaphore. + /// + /// ### Returns + /// * 0 on success. + /// ### Errors + /// * `EBADF (9)`: If the semaphore handle is invalid or the semaphore is + /// already initialized. + /// * `EINVAL (22)`: If the initial value exceeds the maximum allowable + /// value + /// * 'ENOSYS(38)' : Currently not supported + /// for a semaphore (SEM_VALUE_MAX). pub fn sem_init_syscall(&self, sem_handle: u32, pshared: i32, value: u32) -> i32 { // Boundary check @@ -4344,9 +4350,10 @@ impl Cage { let metadata = &SHM_METADATA; let is_shared = pshared != 0; - // Check if a semaphore with the given handle already exists in the semaphore table. - // If it exists, the semaphore is already initialized, so an error is returned. - // This ensures that only new semaphores are initialized. + // Check if a semaphore with the given handle already exists in the semaphore + // table. If it exists, the semaphore is already initialized, so an + // error is returned. This ensures that only new semaphores are + // initialized. let semtable = &self.sem_table; if !semtable.contains_key(&sem_handle) { @@ -4356,7 +4363,8 @@ impl Cage { // Insert the new semaphore into the semaphore table. semtable.insert(sem_handle, new_semaphore.clone()); - // If the semaphore is shared, add it to the shared memory attachments of other processes. + // If the semaphore is shared, add it to the shared memory attachments of other + // processes. if is_shared { let rev_shm = self.rev_shm.lock(); // if its shared and exists in an existing mapping we need to add it to other @@ -4364,16 +4372,20 @@ impl Cage { if let Some((mapaddr, shmid)) = Self::search_for_addr_in_region(&rev_shm, sem_handle) { - let offset = mapaddr - sem_handle; - // iterate through all cages with shared memory segment attached and add semaphor in segments at attached addr + offset - // offset represents the relative position of the semaphore within the shared memory region. - + let offset = mapaddr - sem_handle; + // iterate through all cages with shared memory segment attached and add + // semaphor in segments at attached addr + offset + // offset represents the relative position of the semaphore within the shared + // memory region. + if let Some(segment) = metadata.shmtable.get_mut(&shmid) { for cageid in segment.attached_cages.clone().into_read_only().keys() { let cage = interface::cagetable_getref(*cageid); - // Find all addresses in the shared memory region that belong to the current segment. + // Find all addresses in the shared memory region that belong to the + // current segment. let addrs = Self::rev_shm_find_addrs_by_shmid(&rev_shm, shmid); - // Iterate through all addresses and add the semaphore to the cage's semaphore table. + // Iterate through all addresses and add the semaphore to the cage's + // semaphore table. for addr in addrs.iter() { cage.sem_table.insert(addr + offset, new_semaphore.clone()); } @@ -4389,39 +4401,51 @@ impl Cage { return syscall_error(Errno::EBADF, "sem_init", "semaphore already initialized"); } -/// ## `sem_wait_syscall` -/// -/// ### Description -/// 1. Check for Semaphore Existence:The function first checks if the provided -/// semaphore handle exists in the semaphore table. -/// 2. Acquire Semaphore: If the semaphore exists, the function attempts to acquire it using `lock`. -/// This operation will block the calling process until the semaphore becomes available. -/// 3. Error Handling:If the semaphore handle is invalid, the function returns an error -/// 4. This function allows a process to wait for a semaphore to become available. -/// If the semaphore is currently available (its value is greater than 0), the function will -/// acquire the semaphore and return 0. -/// 5. If the semaphore is unavailable (its value is 0), the function will block the -/// calling process until the semaphore becomes available(its value becomes 1). -///[sem_wait(2)](https://man7.org/linux/man-pages/man3/sem_wait.3.html) -/// ### Function Arguments -/// * `sem_handle`: A unique identifier for the semaphore. -/// -/// ### Returns -/// * 0 on success. -/// ### Errors -/// * `EINVAL(22)`: If the semaphore handle is invalid. -/// * 'EAGAIN(11)' & 'EINTR(4)' currently are not supported + /// ## `sem_wait_syscall` + /// + /// ### Description + /// 1. Check for Semaphore Existence:The function first checks if the + /// provided + /// semaphore handle exists in the semaphore table. + /// 2. Acquire Semaphore: If the semaphore exists, the function attempts to + /// acquire it using `lock`. + /// This operation will block the calling process until the semaphore + /// becomes available. + /// 3. Error Handling:If the semaphore handle is invalid, the function + /// returns an error + /// 4. This function allows a process to wait for a semaphore to become + /// available. + /// If the semaphore is currently available (its value is greater than 0), + /// the function will acquire the semaphore and return 0. + /// 5. If the semaphore is unavailable (its value is 0), the function will + /// block the + /// calling process until the semaphore becomes available(its value becomes + /// 1). [sem_wait(2)](https://man7.org/linux/man-pages/man3/sem_wait.3.html) + /// ### Function Arguments + /// * `sem_handle`: A unique identifier for the semaphore. + /// + /// ### Returns + /// * 0 on success. + /// ### Errors + /// * `EINVAL(22)`: If the semaphore handle is invalid. + /// * 'EAGAIN(11)' & 'EINTR(4)' currently are not supported pub fn sem_wait_syscall(&self, sem_handle: u32) -> i32 { let semtable = &self.sem_table; - // Check whether the semaphore exists in the semaphore table. If found, obtain a mutable borrow to the semaphore entry. + // Check whether the semaphore exists in the semaphore table. If found, obtain a + // mutable borrow to the semaphore entry. if let Some(sementry) = semtable.get_mut(&sem_handle) { - // Clone the semaphore entry to create an independent copy that we can modify without affecting other threads. + // Clone the semaphore entry to create an independent copy that we can modify + // without affecting other threads. let semaphore = sementry.clone(); - // Release the mutable borrow on the original semaphore entry to allow other threads to access the semaphore table concurrently. - // Cloning and dropping the original reference lets us modify the value without deadlocking the dashmap. + // Release the mutable borrow on the original semaphore entry to allow other + // threads to access the semaphore table concurrently. Cloning and + // dropping the original reference lets us modify the value without deadlocking + // the dashmap. drop(sementry); - // Acquire the semaphore. This operation will block the calling process until the - ///semaphore becomes available. The`lock` method internally decrements the semaphore value. + // Acquire the semaphore. This operation will block the calling process until + // the + ///semaphore becomes available. The`lock` method internally + /// decrements the semaphore value. // The lock fun is located in misc.rs semaphore.lock(); } else { @@ -4431,42 +4455,49 @@ impl Cage { return 0; } -/// ## `sem_post_syscall` -/// -/// ### Description -/// This function increments the value of a semaphore. -/// 1. Check for Semaphore Existence:The function first checks if the provided -/// semaphore handle exists in the semaphore table. -/// 2. Increment Semaphore Value: If the semaphore exists, the function -/// increments its value using `unlock`. -/// 3. Error Handling: If the semaphore handle is invalid or incrementing the semaphore -/// would exceed the maximum value, the function returns an appropriate error code. -/// [sem_post](https://man7.org/linux/man-pages/man3/sem_post.3.html) -/// -/// ### Function Arguments -/// * `sem_handle`: A unique identifier for the semaphore. -/// -/// ### Returns -/// * 0 on success. -/// -/// ### Errors -/// * `EINVAL(22)`: If the semaphore handle is invalid. -/// * `EOVERFLOW(75)`: If incrementing the semaphore would exceed the maximum allowable value. + /// ## `sem_post_syscall` + /// + /// ### Description + /// This function increments the value of a semaphore. + /// 1. Check for Semaphore Existence:The function first checks if the + /// provided + /// semaphore handle exists in the semaphore table. + /// 2. Increment Semaphore Value: If the semaphore exists, the function + /// increments its value using `unlock`. + /// 3. Error Handling: If the semaphore handle is invalid or incrementing + /// the semaphore + /// would exceed the maximum value, the function returns an appropriate + /// error code. [sem_post](https://man7.org/linux/man-pages/man3/sem_post.3.html) + /// + /// ### Function Arguments + /// * `sem_handle`: A unique identifier for the semaphore. + /// + /// ### Returns + /// * 0 on success. + /// + /// ### Errors + /// * `EINVAL(22)`: If the semaphore handle is invalid. + /// * `EOVERFLOW(75)`: If incrementing the semaphore would exceed the + /// maximum allowable value. pub fn sem_post_syscall(&self, sem_handle: u32) -> i32 { let semtable = &self.sem_table; // Check whether semaphore exists if let Some(sementry) = semtable.get_mut(&sem_handle) { - // Clone the semaphore entry to create an independent copy that we can modify without affecting other threads + // Clone the semaphore entry to create an independent copy that we can modify + // without affecting other threads let semaphore = sementry.clone(); - // Release the mutable borrow on the original semaphore entry to allow other threads to access the semaphore table concurrently. - // Cloning and dropping the original reference lets us modify the value without deadlocking the dashmap. + // Release the mutable borrow on the original semaphore entry to allow other + // threads to access the semaphore table concurrently. Cloning and + // dropping the original reference lets us modify the value without deadlocking + // the dashmap. drop(sementry); // Increment the semaphore value. - //If the semaphore's value becomes greater than zero, one or more blocked threads will be woken up and - // proceed to acquire the semaphore, decreasing its value. - // The unlock fun is located in misc.rs + //If the semaphore's value becomes greater than zero, one or more blocked + // threads will be woken up and proceed to acquire the semaphore, + // decreasing its value. The unlock fun is located in misc.rs if !semaphore.unlock() { - // Return an error indicating that the maximum allowable value for a semaphore would be exceeded. + // Return an error indicating that the maximum allowable value for a semaphore + // would be exceeded. return syscall_error( Errno::EOVERFLOW, "sem_post", @@ -4479,26 +4510,31 @@ impl Cage { return 0; } -/// ## `sem_destroy_syscall` -/// -/// ### Description -/// This function destroys a semaphore, freeing its associated resources. -/// 1. Check for Semaphore Existence: The function first checks if the provided -/// semaphore handle exists in the semaphore table. -/// 2. Remove from Semaphore Table: If the semaphore exists, the function removes -/// it from the semaphore table. -/// 3. Remove from Shared Memory Attachments (if shared): If the semaphore is shared, the -/// function also removes it from the shared memory attachments of other processes. -/// 4. Error Handling: If the semaphore handle is invalid, the function returns an error. -///[sem_destroy](https://man7.org/linux/man-pages/man3/sem_destroy.3.html) -/// ### Function Arguments -/// * `sem_handle`: A unique identifier for the semaphore. -/// -/// ### Returns -/// * 0 on success. -/// -/// ### Errors -/// * `EINVAL(22)`: If the semaphore handle is invalid. + /// ## `sem_destroy_syscall` + /// + /// ### Description + /// This function destroys a semaphore, freeing its associated resources. + /// 1. Check for Semaphore Existence: The function first checks if the + /// provided + /// semaphore handle exists in the semaphore table. + /// 2. Remove from Semaphore Table: If the semaphore exists, the function + /// removes + /// it from the semaphore table. + /// 3. Remove from Shared Memory Attachments (if shared): If the semaphore + /// is shared, the + /// function also removes it from the shared memory attachments of other + /// processes. + /// 4. Error Handling: If the semaphore handle is invalid, the function + /// returns an error. + ///[sem_destroy](https://man7.org/linux/man-pages/man3/sem_destroy.3.html) + /// ### Function Arguments + /// * `sem_handle`: A unique identifier for the semaphore. + /// + /// ### Returns + /// * 0 on success. + /// + /// ### Errors + /// * `EINVAL(22)`: If the semaphore handle is invalid. pub fn sem_destroy_syscall(&self, sem_handle: u32) -> i32 { let metadata = &SHM_METADATA; @@ -4525,12 +4561,14 @@ impl Cage { for cageid in segment.attached_cages.clone().into_read_only().keys() { // Get a reference to the cagetable for the current cage. let cage = interface::cagetable_getref(*cageid); - // Find all addresses in the shared memory region that belong to the current segment. + // Find all addresses in the shared memory region that belong to the + // current segment. let addrs = Self::rev_shm_find_addrs_by_shmid(&rev_shm, shmid); - // Iterate through all addresses and remove the semaphore from the cage's semaphore table. + // Iterate through all addresses and remove the semaphore from the + // cage's semaphore table. for addr in addrs.iter() { cage.sem_table.remove(&(addr + offset)); //remove semapoores at attached addresses + the offset - //offset represents the relative position of the semaphore within the shared memory region. + //offset represents the relative position of the semaphore within the shared memory region. } } } @@ -4547,34 +4585,39 @@ impl Cage { * Take only sem_t *sem as argument, and return int *sval */ -/// ## `sem_getvalue_syscall` -/// -/// ### Description -/// This function implements the `sem_getvalue` system call, which retrieves the -/// current value of a semaphore. -/// 1. Check for Semaphore Existence: The function first checks if the provided -/// semaphore handle exists in the semaphore table. -/// 2. Retrieve Semaphore Value: If the semaphore exists, the function retrieves -/// its current value and returns it. -/// 3. Error Handling: If the semaphore handle is invalid, the function returns an error. -///[sem_getvalue(2)](https://man7.org/linux/man-pages/man3/sem_getvalue.3.html#:~:text=sem_getvalue()%20places%20the%20current,sem_wait(3)%2C%20POSIX.) -/// -/// ### Function Arguments -/// * `sem_handle`: A unique identifier for the semaphore. -/// -/// ### Returns -/// * The current value of the semaphore on success. -/// -/// ### Errors -/// * `EINVAL(22)`: If the semaphore handle is invalid. + /// ## `sem_getvalue_syscall` + /// + /// ### Description + /// This function implements the `sem_getvalue` system call, which retrieves + /// the current value of a semaphore. + /// 1. Check for Semaphore Existence: The function first checks if the + /// provided + /// semaphore handle exists in the semaphore table. + /// 2. Retrieve Semaphore Value: If the semaphore exists, the function + /// retrieves + /// its current value and returns it. + /// 3. Error Handling: If the semaphore handle is invalid, the function + /// returns an error. + ///[sem_getvalue(2)](https://man7.org/linux/man-pages/man3/sem_getvalue.3.html#:~:text=sem_getvalue()%20places%20the%20current,sem_wait(3)%2C%20POSIX.) + /// + /// ### Function Arguments + /// * `sem_handle`: A unique identifier for the semaphore. + /// + /// ### Returns + /// * The current value of the semaphore on success. + /// + /// ### Errors + /// * `EINVAL(22)`: If the semaphore handle is invalid. pub fn sem_getvalue_syscall(&self, sem_handle: u32) -> i32 { let semtable = &self.sem_table; // Check whether the semaphore exists in the semaphore table. if let Some(sementry) = semtable.get_mut(&sem_handle) { // Clone the semaphore entry to avoid modifying the original entry in the table. let semaphore = sementry.clone(); - // Release the mutable borrow on the original semaphore entry to allow other threads to access the semaphore table concurrently. - // Cloning and dropping the original reference lets us modify the value without deadlocking the dashmap. + // Release the mutable borrow on the original semaphore entry to allow other + // threads to access the semaphore table concurrently. Cloning and + // dropping the original reference lets us modify the value without deadlocking + // the dashmap. drop(sementry); return semaphore.get_value(); } @@ -4585,27 +4628,28 @@ impl Cage { ); } -/// ## `sem_trywait_syscall` -/// -/// ### Description -/// This function implements the `sem_trywait` system call, which attempts -/// to acquire a semaphore without blocking. -/// 1. Check for Semaphore Existence: The function first checks if the -/// provided semaphore handle is valid. -/// 2. Attempt to Acquire: If the semaphore exists, the function attempts -/// to acquire it using `trylock`. -/// 3. Error Handling: If the semaphore is unavailable or the handle is invalid, -/// the function returns an appropriate error code. -/// [sem_trywait(2)](https://man7.org/linux/man-pages/man3/sem_trywait.3p.html) -/// ### Function Arguments -/// * `sem_handle`: A unique identifier for the semaphore. -/// -/// ### Returns -/// * 0 on success (semaphore acquired). -/// -/// ### Errors -/// * `EINVAL(22)`: If the semaphore handle is invalid. -/// * `EAGAIN(11)`: If the semaphore is unavailable (its value is 0). + /// ## `sem_trywait_syscall` + /// + /// ### Description + /// This function implements the `sem_trywait` system call, which attempts + /// to acquire a semaphore without blocking. + /// 1. Check for Semaphore Existence: The function first checks if the + /// provided semaphore handle is valid. + /// 2. Attempt to Acquire: If the semaphore exists, the function attempts + /// to acquire it using `trylock`. + /// 3. Error Handling: If the semaphore is unavailable or the handle is + /// invalid, + /// the function returns an appropriate error code. + /// [sem_trywait(2)](https://man7.org/linux/man-pages/man3/sem_trywait.3p.html) + /// ### Function Arguments + /// * `sem_handle`: A unique identifier for the semaphore. + /// + /// ### Returns + /// * 0 on success (semaphore acquired). + /// + /// ### Errors + /// * `EINVAL(22)`: If the semaphore handle is invalid. + /// * `EAGAIN(11)`: If the semaphore is unavailable (its value is 0). pub fn sem_trywait_syscall(&self, sem_handle: u32) -> i32 { let semtable = &self.sem_table; @@ -4613,13 +4657,17 @@ impl Cage { if let Some(sementry) = semtable.get_mut(&sem_handle) { // Clone the semaphore entry to avoid modifying the original entry in the table. let semaphore = sementry.clone(); - // Release the mutable borrow on the original semaphore entry to allow other threads to access the semaphore table concurrently. - // Cloning and dropping the original reference lets us modify the value without deadlocking the dashmap. + // Release the mutable borrow on the original semaphore entry to allow other + // threads to access the semaphore table concurrently. Cloning and + // dropping the original reference lets us modify the value without deadlocking + // the dashmap. drop(sementry); // Attempt to acquire the semaphore without blocking. - // If the semaphore is currently unavailable (value is 0), this operation will fail. + // If the semaphore is currently unavailable (value is 0), this operation will + // fail. if !semaphore.trylock() { - // Return an error indicating that the operation could not be performed without blocking. + // Return an error indicating that the operation could not be performed without + // blocking. return syscall_error( Errno::EAGAIN, "sem_trywait", @@ -4632,33 +4680,41 @@ impl Cage { // If the semaphore was successfully acquired, return 0. return 0; } - -/// ## `sem_timedwait_syscall` -/// -/// ### Description -/// This function implements the `sem_timedwait` system call, which attempts to -/// acquire a semaphore with a timeout. -/// 1. Convert Timeout to Timespec: The function first converts the provided -/// timeout duration into a `timespec` structure, which is used by the underlying -/// `timedlock` function. -/// 2. Check for Semaphore Existence: The function then checks if the provided -/// semaphore handle exists in the semaphore table. -/// 3. Attempt to Acquire with Timeout: If the semaphore exists, the function attempts -/// to acquire it using `timedlock`, which will block for the specified duration. -/// 4. Error Handling: If the semaphore is unavailable, the timeout expires, -/// or the handle is invalid, the function returns an appropriate error code. -///[sem_timedwait(2)](https://man7.org/linux/man-pages/man3/sem_timedwait.3p.html) -/// ### Function Arguments -/// * `sem_handle`: A unique identifier for the semaphore. -/// * `time`: The maximum time to wait for the semaphore to become available, -/// expressed as a `RustDuration`. -/// -/// ### Returns -/// * 0 on success (semaphore acquired). -/// -/// ### Errors -/// * `ETIMEDOUT(110)`: If the timeout expires before the semaphore becomes available. -/// * `EINVAL(22)`: If the semaphore handle is invalid or the timeout value is invalid. + + /// ## `sem_timedwait_syscall` + /// + /// ### Description + /// This function implements the `sem_timedwait` system call, which attempts + /// to acquire a semaphore with a timeout. + /// 1. Convert Timeout to Timespec: The function first converts the + /// provided + /// timeout duration into a `timespec` structure, which is used by the + /// underlying `timedlock` function. + /// 2. Check for Semaphore Existence: The function then checks if the + /// provided + /// semaphore handle exists in the semaphore table. + /// 3. Attempt to Acquire with Timeout: If the semaphore exists, the + /// function attempts + /// to acquire it using `timedlock`, which will block for the specified + /// duration. + /// 4. Error Handling: If the semaphore is unavailable, the timeout + /// expires, + /// or the handle is invalid, the function returns an appropriate error + /// code. [sem_timedwait(2)](https://man7.org/linux/man-pages/man3/sem_timedwait.3p.html) + /// ### Function Arguments + /// * `sem_handle`: A unique identifier for the semaphore. + /// * `time`: The maximum time to wait for the semaphore to become + /// available, + /// expressed as a `RustDuration`. + /// + /// ### Returns + /// * 0 on success (semaphore acquired). + /// + /// ### Errors + /// * `ETIMEDOUT(110)`: If the timeout expires before the semaphore becomes + /// available. + /// * `EINVAL(22)`: If the semaphore handle is invalid or the timeout value + /// is invalid. pub fn sem_timedwait_syscall(&self, sem_handle: u32, time: interface::RustDuration) -> i32 { let abstime = libc::timespec { tv_sec: time.as_secs() as i64, @@ -4670,14 +4726,18 @@ impl Cage { let semtable = &self.sem_table; // Check whether semaphore exists if let Some(sementry) = semtable.get_mut(&sem_handle) { - // Clone the semaphore entry to create an independent copy that we can modify without affecting other threads + // Clone the semaphore entry to create an independent copy that we can modify + // without affecting other threads let semaphore = sementry.clone(); - // Release the mutable borrow on the original semaphore entry to allow other threads to access the semaphore table concurrently. - // Cloning and dropping the original reference lets us modify the value without deadlocking the dashmap. + // Release the mutable borrow on the original semaphore entry to allow other + // threads to access the semaphore table concurrently. Cloning and + // dropping the original reference lets us modify the value without deadlocking + // the dashmap. drop(sementry); // Attempt to acquire the semaphore with a timeout. if !semaphore.timedlock(time) { - // Return an error indicating that the call timed out before the semaphore could be locked. + // Return an error indicating that the call timed out before the semaphore could + // be locked. return syscall_error( Errno::ETIMEDOUT, "sem_timedwait", diff --git a/src/tests/fs_tests.rs b/src/tests/fs_tests.rs index c909df65c..d0d7e0753 100644 --- a/src/tests/fs_tests.rs +++ b/src/tests/fs_tests.rs @@ -1607,8 +1607,8 @@ pub mod fs_tests { //their execution using a shared semaphore. The test aims to ensure: // 1. The semaphore is initialized correctly. // 2. The child process can acquire and release the semaphore. - // 3. The parent process can acquire and release the - // semaphore after the child process exits. + // 3. The parent process can acquire and release the semaphore after the child + // process exits. // 4. The semaphore can be destroyed safely. #[test] pub fn ut_lind_fs_sem_fork() { @@ -1621,10 +1621,10 @@ pub mod fs_tests { // Create a shared memory region of 1024 bytes. This region will be // shared between the parent and child process. - // IPC_CREAT tells the system to create a new memory segment for the shared memory - // and 0666 sets the access permissions of the memory segment. + // IPC_CREAT tells the system to create a new memory segment for the shared + // memory and 0666 sets the access permissions of the memory segment. let shmid = cage.shmget_syscall(key, 1024, 0666 | IPC_CREAT); - + // Attach shared memory for semaphore access. let shmatret = cage.shmat_syscall(shmid, 0xfffff000 as *mut u8, 0); assert_ne!(shmatret, -1); @@ -1639,7 +1639,7 @@ pub mod fs_tests { // Set reference to child process's cagetable (ID 2) for independent operation. let cage1 = interface::cagetable_getref(2); // Child process blocks on semaphore wait (decrementing it from 1 to 0). - assert_eq!(cage1.sem_wait_syscall(shmatret as u32), 0); + assert_eq!(cage1.sem_wait_syscall(shmatret as u32), 0); // Simulate processing time with 40ms delay. interface::sleep(interface::RustDuration::from_millis(40)); // Child process releases semaphore, signaling its availability to parent @@ -1648,14 +1648,17 @@ pub mod fs_tests { cage1.exit_syscall(EXIT_SUCCESS); }); - // Parent waits on semaphore (blocks until released by child, decrementing to 0). + // Parent waits on semaphore (blocks until released by child, decrementing to + // 0). assert_eq!(cage.sem_wait_syscall(shmatret as u32), 0); assert_eq!(cage.sem_getvalue_syscall(shmatret as u32), 0); - // Simulate parent process processing time with 100ms delay to ensure synchronization. + // Simulate parent process processing time with 100ms delay to ensure + // synchronization. interface::sleep(interface::RustDuration::from_millis(100)); - // Wait for child process to finish to prevent race conditions before destroying semaphore. - //Release semaphore, making it available again (value increases to 1). - assert_eq!(cage.sem_post_syscall(shmatret as u32), 0); + // Wait for child process to finish to prevent race conditions before destroying + // semaphore. Release semaphore, making it available again (value + // increases to 1). + assert_eq!(cage.sem_post_syscall(shmatret as u32), 0); thread_child.join().unwrap(); // Destroy the semaphore @@ -1672,11 +1675,13 @@ pub mod fs_tests { } // This test verifies the functionality of timed semaphores in a fork scenario. - // It involves a parent process and a child process that synchronize their execution using a - //shared semaphore with a timeout. The test aims to ensure: + // It involves a parent process and a child process that synchronize their + // execution using a shared semaphore with a timeout. The test aims to + // ensure: // 1. The semaphore is initialized correctly. // 2. The child process can acquire and release the semaphore. - // 3. The parent process can acquire the semaphore using a timed wait operation with a + // 3. The parent process can acquire the semaphore using a timed wait operation + // with a // timeout, and the semaphore is acquired successfully. // 4. The parent process can release the semaphore. // 5. The semaphore can be destroyed safely. @@ -1690,8 +1695,8 @@ pub mod fs_tests { let key = 31337; // Create a shared memory region of 1024 bytes. //This region will be shared between the parent and child process. - // IPC_CREAT tells the system to create a new memory segment for the shared memory - // and 0666 sets the access permissions of the memory segment. + // IPC_CREAT tells the system to create a new memory segment for the shared + // memory and 0666 sets the access permissions of the memory segment. let shmid = cage.shmget_syscall(key, 1024, 0666 | IPC_CREAT); // Attach the shared memory region to the address space of the process // to make sure for both processes to access the shared semaphore. @@ -1701,7 +1706,8 @@ pub mod fs_tests { let ret_init = cage.sem_init_syscall(shmatret as u32, 1, 1); assert_eq!(ret_init, 0); assert_eq!(cage.sem_getvalue_syscall(shmatret as u32), 1); - // Fork process, creating a child process with its own independent cagetable (ID 2). + // Fork process, creating a child process with its own independent cagetable (ID + // 2). assert_eq!(cage.fork_syscall(2), 0); // Define the child process behavior in a separate thread let thread_child = interface::helper_thread(move || { @@ -1712,7 +1718,8 @@ pub mod fs_tests { assert_eq!(cage1.sem_wait_syscall(shmatret as u32), 0); // Simulate some work by sleeping for 20 milliseconds. interface::sleep(interface::RustDuration::from_millis(20)); - // Child process releases semaphore, signaling its availability to the parent process + // Child process releases semaphore, signaling its availability to the parent + // process //(value increases from 0 to 1). assert_eq!(cage1.sem_post_syscall(shmatret as u32), 0); cage1.exit_syscall(EXIT_SUCCESS); From 605edf1264fe12f8369ce231710ef558e499d8a7 Mon Sep 17 00:00:00 2001 From: lind Date: Mon, 1 Jul 2024 23:52:32 +0000 Subject: [PATCH 8/8] fixed according to the review --- src/safeposix/syscalls/fs_calls.rs | 61 +++++++++++++----------------- src/tests/fs_tests.rs | 9 ----- 2 files changed, 26 insertions(+), 44 deletions(-) diff --git a/src/safeposix/syscalls/fs_calls.rs b/src/safeposix/syscalls/fs_calls.rs index cd402b0b4..621be28cf 100644 --- a/src/safeposix/syscalls/fs_calls.rs +++ b/src/safeposix/syscalls/fs_calls.rs @@ -3624,9 +3624,8 @@ impl Cage { /// /// * `EINVAL` - the bufsize argument is zero and buf is not a NULL pointer. /// * `ERANGE` - the bufsize argument is less than the length of the - /// absolute - /// pathname of the working directory, including the terminating null byte. - /// * `EFAULT` - buf points to a bad address. + /// absolute pathname of the working directory, including the + /// terminating null byte. /// Other errors, like `EACCES`, `ENOMEM`, etc. are not supported. /// /// ### Panics @@ -3637,39 +3636,31 @@ impl Cage { /// [getcwd(3)](https://man7.org/linux/man-pages/man3/getcwd.3.html) pub fn getcwd_syscall(&self, buf: *mut u8, bufsize: u32) -> i32 { - //The first two conditions help to error-out quickly if either - //the pointer to the string is a null pointer or the specified - //size of the string is 0. - if !buf.is_null() { - if (bufsize as usize) == 0 { - return syscall_error(Errno::EINVAL, "getcwd", "size of the specified buffer is 0"); - } else { - //Cages store their current working directory as path buffers. - //To use the obtained directory as a string, a null terminator needs - //to be added to the path. - let mut bytes: Vec = self.cwd.read().to_str().unwrap().as_bytes().to_vec(); - bytes.push(0u8); //Adding a null terminator to the end of the string - let length = bytes.len(); - //The bufsize argument should be at least the length of the absolute - //pathname of the working directory, including the terminating null byte. - if (bufsize as usize) < length { - return syscall_error(Errno::ERANGE, "getcwd", "the length (in bytes) of the absolute pathname of the current working directory exceeds the given size"); - } - //It is expected that only the first `bufsize` bytes of the `buf` string - //will be written into. The `fill()` function ensures this by taking - //a mutable slice of length `bufsize` to the string pointed to by `buf` - //and inserting the obtained current working directory into that slice, - //thus prohibiting writing into the remaining bytes of the string. - interface::fill(buf, length, &bytes); - //returning 0 on success - 0 - } + //Here we only check if the size of the specified + //string is 0. Null pointers are handled beforehand + //by nacl and `types.rs`. + if (bufsize as usize) == 0 { + return syscall_error(Errno::EINVAL, "getcwd", "size of the specified buffer is 0"); } else { - return syscall_error( - Errno::EFAULT, - "getcwd", - "an invalid (null) pointer to the buffer is provided", - ); + //Cages store their current working directory as path buffers. + //To use the obtained directory as a string, a null terminator needs + //to be added to the path. + let mut bytes: Vec = self.cwd.read().to_str().unwrap().as_bytes().to_vec(); + bytes.push(0u8); //Adding a null terminator to the end of the string + let length = bytes.len(); + //The bufsize argument should be at least the length of the absolute + //pathname of the working directory, including the terminating null byte. + if (bufsize as usize) < length { + return syscall_error(Errno::ERANGE, "getcwd", "the length (in bytes) of the absolute pathname of the current working directory exceeds the given size"); + } + //It is expected that only the first `bufsize` bytes of the `buf` string + //will be written into. The `fill()` function ensures this by taking + //a mutable slice of length `bufsize` to the string pointed to by `buf` + //and inserting the obtained current working directory into that slice, + //thus prohibiting writing into the remaining bytes of the string. + interface::fill(buf, length, &bytes); + //returning 0 on success + 0 } } diff --git a/src/tests/fs_tests.rs b/src/tests/fs_tests.rs index d0d7e0753..cf51f5457 100644 --- a/src/tests/fs_tests.rs +++ b/src/tests/fs_tests.rs @@ -1468,15 +1468,6 @@ pub mod fs_tests { assert_eq!(cage.access_syscall("newcwd2", F_OK), 0); assert_eq!(cage.chdir_syscall("newcwd2"), 0); - //Checking if passing a null pointer and the correct string size - //to `getcwd_syscall()`correctly results in `Buf points to - //a bad address` error - let null_ptr: *mut u8 = std::ptr::null_mut(); - assert_eq!( - cage.getcwd_syscall(null_ptr, newcwdsize_u32), - -(Errno::EFAULT as i32) - ); - //Checking if passing a valid string pointer and a size of 0 to //`getcwd_syscall()` correctly results in `The size argument is zero and //buf is not a null pointer` error