Dup/dup2 syscall tests and comments (#285)

rupeshkoushik07 · lind · commit 5223a1513864 · 2024-07-15T17:52:13.000Z
* add tests

* update dup2 max

* change error

* update the comments and fromatting

* update files/streams

* add test

* remove

* remove

* remove import
diff --git a/src/safeposix/syscalls/fs_calls.rs b/src/safeposix/syscalls/fs_calls.rs
@@ -2230,7 +2230,44 @@ impl Cage {
         0 //chdir has succeeded!;
     }
 
-    //------------------------------------DUP & DUP2 SYSCALLS------------------------------------
+    ///##------------------------------------DUP & DUP2 SYSCALLS------------------------------------
+    /// ## `dup_syscall`
+    ///
+    /// ### Description
+    /// This function duplicates a file descriptor. It creates a new file
+    /// descriptor that refers to the same open file description as the original file descriptor.
+    /// * Finding the Next Available File Descriptor: If `start_desc` is provided and it is already in use, the function
+    ///   will continue searching for the next available file descriptor starting from `start_desc`. If no file
+    ///   descriptors are available, it will return an error (`ENFILE`).
+    /// * If `fd` is equal to `start_fd`, the function returns `start_fd` as the new file
+    ///   descriptor. This is because in this scenario, the original and new file descriptors would point to the same
+    ///   file description.
+    /// * The `_dup2_helper` function is called to perform the actual file descriptor duplication, handling the
+    ///   allocation of a new file descriptor, updating the file descriptor table, and incrementing the reference count
+    ///   of the file object.
+    /// * The function modifies the global `filedescriptortable` array, adding a new entry for the
+    ///   duplicated file descriptor. It also increments the reference count of the file object associated with the
+    ///   original file descriptor.
+    /// * The `false` argument passed to `_dup2_helper` indicates that this call is from the `dup_syscall` function,
+    ///   not the `dup2_syscall` function.
+    ///
+    /// ### Function Arguments
+    /// * `fd`: The original file descriptor to duplicate.
+    /// * `start_desc`:  An optional starting file descriptor number. If provided, the new file descriptor will be
+    ///  assigned the first available file descriptor number starting from this value. If not provided, it defaults to
+    ///  `STARTINGFD`,which is the minimum designated file descriptor value for new file descriptors.
+    ///
+    /// ### Returns
+    /// * The new file descriptor on success.
+    /// * `EBADF`: If the original file descriptor is invalid.
+    /// * `ENFILE`: If there are no available file descriptors.
+    ///
+    /// ### Errors
+    /// * `EBADF(9)`: If the original file descriptor is invalid.
+    /// * `ENFILE(23)`: If there are no available file descriptors.
+    ///  ###Panics
+    /// * There are no panics for this syscall
+    ///[dup(2)](https://man7.org/linux/man-pages/man2/dup.2.html)
 
     pub fn dup_syscall(&self, fd: i32, start_desc: Option<i32>) -> i32 {
         //if a starting fd was passed, then use that as the starting point, but
@@ -2245,7 +2282,11 @@ impl Cage {
         } //if the file descriptors are equal, return the new one
 
         // get the filedesc_enum
-        let checkedfd = self.get_filedescriptor(fd).unwrap();
+        // Attempt to get the file descriptor; handle error if it does not exist
+        let checkedfd = match self.get_filedescriptor(fd) {
+            Ok(fd) => fd,
+            Err(_) => return syscall_error(Errno::EBADF, "dup", "Invalid old file descriptor."),
+        };
         let filedesc_enum = checkedfd.write();
         let filedesc_enum = if let Some(f) = &*filedesc_enum {
             f
@@ -2257,6 +2298,34 @@ impl Cage {
         return Self::_dup2_helper(&self, filedesc_enum, start_fd, false);
     }
 
+    /// ## `dup2_syscall`
+    ///
+    /// ### Description
+    /// This function implements the `dup2` system call, which duplicates a file descriptor and assigns it to a new file
+    /// descriptor number. If the new file descriptor already exists, it is closed before the duplication takes place.
+    /// * File Descriptor Reuse:  If the new file descriptor (`newfd`) is already open, the function will first close the
+    ///   existing file descriptor silently (without returning an error) before allocating a new file descriptor and
+    ///   updating the file descriptor table.
+    /// * If `oldfd` and `newfd` are the same, the function returns `newfd` without closing it.
+    ///   This is because in this scenario, the original and new file descriptors would already point to the same file
+    ///   description.
+    /// * the global `filedescriptortable` array, replacing the entry for the
+    ///   new file descriptor with a new entry for the duplicated file descriptor. It also increments the reference count of the
+    ///   file object associated with the original file descriptor.
+    ///
+    /// ### Function Arguments
+    /// * `oldfd`: The original file descriptor to duplicate.
+    /// * `newfd`: The new file descriptor number to assign to the duplicated file descriptor.
+    ///
+    /// ### Returns
+    /// * The new file descriptor on success.
+    ///
+    /// ### Errors
+    /// * `EBADF(9)`: If the original file descriptor (`oldfd`) is invalid or the new file descriptor (`newfd`) number is out of range.
+    ///  ###Panics
+    /// * There are no panics for this syscall
+    ///[dup2(2)](https://linux.die.net/man/2/dup2)
+
     pub fn dup2_syscall(&self, oldfd: i32, newfd: i32) -> i32 {
         //checking if the new fd is out of range
         if newfd >= MAXFD || newfd < 0 {
@@ -2272,7 +2341,10 @@ impl Cage {
         } //if the file descriptors are equal, return the new one
 
         // get the filedesc_enum
-        let checkedfd = self.get_filedescriptor(oldfd).unwrap();
+        let checkedfd = match self.get_filedescriptor(oldfd) {
+            Ok(fd) => fd,
+            Err(_) => return syscall_error(Errno::EBADF, "dup2", "Invalid old file descriptor."),
+        };
         let filedesc_enum = checkedfd.write();
         let filedesc_enum = if let Some(f) = &*filedesc_enum {
             f
@@ -2284,6 +2356,41 @@ impl Cage {
         return Self::_dup2_helper(&self, filedesc_enum, newfd, true);
     }
 
+    /// ## `_dup2_helper`
+    ///
+    /// ### Description
+    /// This helper function performs the actual file descriptor duplication process for both `dup` and `dup2` system calls.
+    /// It handles the allocation of a new file descriptor, updates the file descriptor table, and increments the reference count of the
+    /// associated file object.
+    /// * Duplication from `dup2_syscall`: If `fromdup2` is true, the function first closes the existing file descriptor
+    ///   at `newfd` (if any) before allocating a new file descriptor and updating the file descriptor table.
+    /// * Duplication from `dup_syscall`: If `fromdup2` is false, the function allocates a new file descriptor, finds the
+    ///   first available file descriptor number starting from `newfd`, and updates the file descriptor table.
+    /// * Reference Counting: The function increments the reference count of the file object associated with the original file
+    ///   descriptor. This ensures that the file object is not deleted until all its associated file descriptors are closed.
+    /// * Socket Handling: For domain sockets, the function increments the reference count of both the send and receive pipes
+    ///   associated with the socket.
+    /// * Stream Handling: Streams are not currently supported for duplication
+    /// * Unhandled File Types: If the file descriptor is associated with a file type that is not handled by the function (i.e.,
+    ///   not a File, Pipe, Socket, or Stream), the function returns an error (`EACCES`).
+    /// * The function does not handle streams.
+    /// * Socket Handling: If the file descriptor is associated with a socket, the function handles domain sockets differently
+    ///   by incrementing the reference count of both the send and receive pipes.
+    /// ### Function Arguments
+    /// * `self`:  A reference to the `FsCalls` struct, which contains the file descriptor table and other system-related data.
+    /// * `filedesc_enum`: A reference to the `FileDescriptor` object representing the file descriptor to be duplicated.
+    /// * `newfd`: The new file descriptor number to assign to the duplicated file descriptor.
+    /// * `fromdup2`: A boolean flag indicating whether the call is from `dup2_syscall` (true) or `dup_syscall` (false).
+    ///
+    /// ### Returns
+    /// * The new file descriptor on success.
+    ///
+    /// ### Errors
+    /// * `ENFILE(23)`: If there are no available file descriptors.
+    /// * `EACCES(13)`: If the file descriptor cannot be duplicated.
+    /// ###Panics
+    /// * If the file descriptor is associated with a socket, and the inode does not match the file descriptor.
+
     pub fn _dup2_helper(&self, filedesc_enum: &FileDescriptor, newfd: i32, fromdup2: bool) -> i32 {
         let (dupfd, mut dupfdguard) = if fromdup2 {
             let mut fdguard = self.filedescriptortable[newfd as usize].write();
@@ -2301,6 +2408,8 @@ impl Cage {
         } else {
             let (newdupfd, guardopt) = self.get_next_fd(Some(newfd));
             if newdupfd < 0 {
+                // The function allocates a new file descriptor and updates the file descriptor table,
+                // handling the potential for file descriptor table overflow (resulting in an `ENFILE` error).
                 return syscall_error(
                     Errno::ENFILE,
                     "dup2_helper",
@@ -2319,6 +2428,8 @@ impl Cage {
                 //incrementing the ref count so that when close is executed on the dup'd file
                 //the original file does not get a negative ref count
                 match *inodeobj {
+                    // increments the reference count of the file object associated with the original file descriptor
+                    // to ensure that the file object is not deleted until all its associated file descriptors are closed.
                     Inode::File(ref mut normalfile_inode_obj) => {
                         normalfile_inode_obj.refcount += 1;
                     }
diff --git a/src/tests/fs_tests.rs b/src/tests/fs_tests.rs
@@ -495,6 +495,46 @@ pub mod fs_tests {
         assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
         lindrustfinalize();
     }
+    #[test]
+    fn ut_lind_fs_dup_invalid_fd() {
+        let _thelock = setup::lock_and_init();
+        let cage = interface::cagetable_getref(1);
+
+        // Open a file and get a valid file descriptor
+        let fd = cage.open_syscall("/testfile", O_CREAT | O_WRONLY, S_IRWXA);
+        assert_ne!(fd, -(Errno::ENOENT as i32));
+
+        // Close the file descriptor, making it invalid
+        assert_eq!(cage.close_syscall(fd), 0);
+
+        // Attempt to duplicate the invalid file descriptor
+        let new_fd = cage.dup_syscall(fd, None);
+        assert_eq!(new_fd, -(Errno::EBADF as i32));
+
+        assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
+        lindrustfinalize();
+    }
+
+    #[test]
+    fn ut_lind_fs_dup_full_table() {
+        let _thelock = setup::lock_and_init();
+        let cage = interface::cagetable_getref(1);
+
+        // Open a large number of files to fill the file descriptor table
+        for i in 0..1024 {
+            let fd = cage.open_syscall(&format!("/testfile{}", i), O_CREAT | O_WRONLY, S_IRWXA);
+            assert_ne!(fd, -(Errno::ENOENT as i32));
+        }
+
+        // Attempt to duplicate a file descriptor, which should fail
+        let fd = cage.open_syscall("/testfile", O_CREAT | O_WRONLY, S_IRWXA);
+        assert_ne!(fd, -(Errno::ENOENT as i32));
+        let new_fd = cage.dup_syscall(fd, None);
+        assert_eq!(new_fd, -(Errno::EBADF as i32));
+
+        assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
+        lindrustfinalize();
+    }
 
     #[test]
     pub fn ut_lind_fs_dup2() {
@@ -548,6 +588,97 @@ pub mod fs_tests {
         lindrustfinalize();
     }
 
+    #[test]
+    fn ut_lind_fs_dup2_invalid_fd() {
+        let _thelock = setup::lock_and_init();
+        let cage = interface::cagetable_getref(1);
+
+        // Open a file
+        let fd = cage.open_syscall("/testfile", O_CREAT | O_WRONLY, S_IRWXA);
+        assert_ne!(fd, -(Errno::ENOENT as i32));
+
+        // Close the file descriptor, making it invalid
+        assert_eq!(cage.close_syscall(fd), 0);
+
+        // Attempt to duplicate the invalid file descriptor
+        let new_fd = cage.dup2_syscall(fd, 5);
+        assert_eq!(new_fd, -(Errno::EBADF as i32));
+
+        assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
+        lindrustfinalize();
+    }
+
+    #[test]
+    fn ut_lind_fs_dup2_full_table() {
+        let _thelock = setup::lock_and_init();
+        let cage = interface::cagetable_getref(1);
+
+        // Open a large number of files to fill the file descriptor table
+        for i in 0..1024 {
+            let fd = cage.open_syscall(&format!("/testfile{}", i), O_CREAT | O_WRONLY, S_IRWXA);
+            assert_ne!(fd, -(Errno::ENOENT as i32));
+        }
+
+        // Attempt to duplicate a file descriptor, which should fail
+        let fd = cage.open_syscall("/testfile", O_CREAT | O_WRONLY, S_IRWXA);
+        assert_ne!(fd, -(Errno::ENOENT as i32));
+        let new_fd = cage.dup2_syscall(fd, 5); // Try to duplicate to an existing fd
+        assert_eq!(new_fd, -(Errno::EBADF as i32));
+
+        assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
+        lindrustfinalize();
+    }
+
+    #[test]
+    fn ut_lind_fs_dup2_with_fork() {
+        // 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 flags: i32 = O_CREAT | O_RDWR;
+        let filepath1 = "/dup2file_with_fork1";
+        let filepath2 = "/dup2file_with_fork2";
+    
+        // Open file descriptors
+        let fd1 = cage.open_syscall(filepath1, flags, S_IRWXA);
+        let fd2 = cage.open_syscall(filepath2, flags, S_IRWXA);
+        assert!(fd1 >= 0);
+        assert!(fd2 >= 0);
+    
+        // Write data to the first file
+        assert_eq!(cage.write_syscall(fd1, str2cbuf("parent data"), 11), 11);
+    
+        // Fork the process
+        assert_eq!(cage.fork_syscall(2), 0);
+    
+        let child = std::thread::spawn(move || {
+            let cage2 = interface::cagetable_getref(2);
+    
+            // In the child process, duplicate fd1 to fd2
+            assert!(cage2.dup2_syscall(fd1, fd2) >= 0);
+    
+            // Write new data to the duplicated file descriptor
+            assert_eq!(cage2.write_syscall(fd2, str2cbuf(" child data"), 11), 11);
+    
+            assert_eq!(cage2.close_syscall(fd2), 0);
+    
+            assert_eq!(cage2.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
+        });
+    
+        child.join().unwrap();
+    
+        let mut buffer = sizecbuf(22);
+        assert_eq!(cage.lseek_syscall(fd1, 0, SEEK_SET), 0);
+        assert_eq!(cage.read_syscall(fd1, buffer.as_mut_ptr(), 22), 22);
+        assert_eq!(cbuf2str(&buffer), "parent data child data");
+    
+        assert_eq!(cage.close_syscall(fd1), 0);
+    
+        assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS);
+        lindrustfinalize();
+    }    
+
     #[test]
     pub fn ut_lind_fs_fcntl_valid_args() {
         //acquiring a lock on TESTMUTEX prevents other tests from running concurrently,
