From c92ac2c57b48ae2e17abd23f10b30203b4480242 Mon Sep 17 00:00:00 2001 From: Naman Lalit Date: Fri, 26 Jul 2024 14:55:18 -0400 Subject: [PATCH 1/5] lseek_syscall and close_syscall updates (#311) * Added comments for lseek syscall * Added tests for lseek syscall * Added comments for close_syscall * Added tests for close syscall * Addressed review comments * Addressed review comments * Addressed review comment --------- Co-authored-by: Nicholas Renner --- src/safeposix/syscalls/fs_calls.rs | 374 +++++++++++++++++++++++------ src/tests/fs_tests.rs | 328 +++++++++++++++++++++++++ 2 files changed, 631 insertions(+), 71 deletions(-) diff --git a/src/safeposix/syscalls/fs_calls.rs b/src/safeposix/syscalls/fs_calls.rs index b0ba36e7b..bfd04ebc3 100644 --- a/src/safeposix/syscalls/fs_calls.rs +++ b/src/safeposix/syscalls/fs_calls.rs @@ -2443,31 +2443,123 @@ impl Cage { } } - //------------------------------------LSEEK SYSCALL------------------------------------ + /// ## -------------------------------- LSEEK SYSCALL ------------------------- + /// ### Description + /// + /// The `lseek_syscall()` repositions the offset of the file associated + /// with the file descriptor `fd` to a new location as specified by + /// `offset` and `whence`. The new offset is calculated relative to the + /// position specified by whence. This syscall allows the file offset to + /// be set beyond the end of the file (but this does not change the size + /// of the file). If data is later written at this point, subsequent reads + /// of the data in the gap (a "hole") return null bytes ('\0') until data + /// is actually written into the gap. + /// + /// ### Function Arguments + /// + /// The `lseek_syscall()` receives three arguments: + /// * `fd` - It is the file descriptor, which identifies the file being + /// accessed + /// * `offset` - It determines the position in the file where the file + /// offset should be set. It specifies the number of bytes from a + /// particular reference point. + /// * `whence` - It determines how the seek operation is performed. There + /// are three possible values for whence: `SEEK_SET`: The file offset is + /// set to 0 + offset bytes. `SEEK_CUR`: The file offset is set to its + /// current location plus offset bytes. `SEEK_END`: The file offset is set + /// to the size of the file plus offset bytes. + /// + /// ### Returns + /// + /// Upon successful completion of this call, we return the resulting offset + /// location as measured in bytes from the beginning of the file. + /// Otherwise, errors or panics are returned for different scenarios. + /// + /// ### Errors + /// + /// * `EINVAL` - The `whence` type is different from the available values; + /// seek to a position before the start of the file; seek to after last + /// position in directory + /// * `ESPIPE` - Seek is not possible when file descriptor is a + /// socket/pipe/stream/epoll. + /// + /// + /// ### Panics + /// + /// * When there is some issue fetching the file descriptor. + /// * When the file type file descriptor contains a Socket as an inode. + /// + /// For more detailed description of all the commands and return values, see + /// [lseek(2)](https://man7.org/linux/man-pages/man2/lseek.2.html) pub fn lseek_syscall(&self, fd: i32, offset: isize, whence: i32) -> i32 { + //BUG + //If the provided file descriptor is out of bounds, get_filedescriptor returns + //Err(), unwrapping on which produces a 'panic!' + //otherwise, file descriptor table entry is stored in 'checkedfd' let checkedfd = self.get_filedescriptor(fd).unwrap(); + // Acquire a write lock on the file descriptor to ensure exclusive access. let mut unlocked_fd = checkedfd.write(); + + // Check if the file descriptor object is valid if let Some(filedesc_enum) = &mut *unlocked_fd { //confirm fd type is seekable match filedesc_enum { + // Return an error for Sockets, as they do not support the concept of seeking to a + // specific offset because data arrives in a continuous stream from the network. + Socket(_) => syscall_error( + Errno::ESPIPE, + "lseek", + "file descriptor is associated with a socket, cannot seek", + ), + // Return an error for Streams, as like sockets, streams are sequential and do not + // support seeking to an offset. + Stream(_) => syscall_error( + Errno::ESPIPE, + "lseek", + "file descriptor is associated with a stream, cannot seek", + ), + // Return an error for Pipes, as they are designed for sequential reads and writes + // between processes. Seeking within a pipe would not be possible. + Pipe(_) => syscall_error( + Errno::ESPIPE, + "lseek", + "file descriptor is associated with a pipe, cannot seek", + ), + // Return an error for Epoll, as Epoll file descriptors are for event notification + // and do not hold any data themselves, so seeking is not possible. + Epoll(_) => syscall_error( + Errno::ESPIPE, + "lseek", + "file descriptor is associated with an epollfd, cannot seek", + ), File(ref mut normalfile_filedesc_obj) => { + // Get the inode object from the inode table associated with the file + // descriptor. let inodeobj = FS_METADATA .inodetable .get(&normalfile_filedesc_obj.inode) .unwrap(); - //handle files/directories differently + // Match the type of inode object with the type (File, Socket, CharDev, Dir) + // Handle files/directories differently match &*inodeobj { Inode::File(normalfile_inode_obj) => { + // For file type inode, match the type of whence first, and based + // on that, update the final position of the offset of the file. let eventualpos = match whence { + // Simply update the offset of the file to the given value. SEEK_SET => offset, + // Get the current offset position of the file and add `offset`. SEEK_CUR => normalfile_filedesc_obj.position as isize + offset, + // Get the file size and add `offset` position. SEEK_END => normalfile_inode_obj.size as isize + offset, _ => { return syscall_error(Errno::EINVAL, "lseek", "unknown whence"); } }; + // Sanity check to make sure that the final position of seeking + // doesn't go before position 0 in the file. if eventualpos < 0 { return syscall_error( Errno::EINVAL, @@ -2475,30 +2567,42 @@ impl Cage { "seek to before position 0 in file", ); } - //subsequent writes to the end of the file must zero pad up until this - // point if we overran the end of our file - // when seeking + // subsequent writes to the end of the file must zero pad up (filling + // the gap between the current end of the file and the new position with + // zero bytes ('\0')) until this point if we overran the end of our file + // when seeking. This is currently supported in write and pwrite + // syscalls. + // Update the final position of the file descriptor object normalfile_filedesc_obj.position = eventualpos as usize; - //return the location that we sought to + // Return the location that we sought to eventualpos as i32 } Inode::CharDev(_) => { - 0 //for character files, rather than seeking, we + 0 // for character files, rather than seeking, we // transparently do nothing } + // A Sanity check is added to make sure that there is no such case when the + // fd type is "File" and the inode type is "Socket". This state is ideally + // not possible, so we panic in such cases. Inode::Socket(_) => { panic!("lseek: socket fd and inode don't match types") } Inode::Dir(dir_inode_obj) => { - //for directories we seek between entries, and thus our end position is - // the total number of entries + // For directory type inode, we seek between directory entries, + // and based on the whence check, we update the final position. let eventualpos = match whence { + // Simply update the offset of the directory to the given value. SEEK_SET => offset, + // Get the current position of the directory pointing to a directory + // entry and add the offset to it. SEEK_CUR => normalfile_filedesc_obj.position as isize + offset, + // The Inode dictionary contains the directory entries mapped with + // the inode object, and final position is updated by adding the + // total count of the entries with the given offset. SEEK_END => { dir_inode_obj.filename_to_inode_dict.len() as isize + offset } @@ -2507,7 +2611,8 @@ impl Cage { } }; - //confirm that the location we want to seek to is valid + // Sanity check to make sure that the final position of seeking + // doesn't go before position 0 in the directory. if eventualpos < 0 { return syscall_error( Errno::EINVAL, @@ -2515,6 +2620,8 @@ impl Cage { "seek to before position 0 in directory", ); } + // Return an error if the position we are seeking is after the + // last possible position in the directory. if eventualpos > dir_inode_obj.filename_to_inode_dict.len() as isize { return syscall_error( Errno::EINVAL, @@ -2523,32 +2630,13 @@ impl Cage { ); } + // Update the final position of the file descriptor object normalfile_filedesc_obj.position = eventualpos as usize; - //return the location that we sought to + // Return the location that we sought to eventualpos as i32 } } } - Socket(_) => syscall_error( - Errno::ESPIPE, - "lseek", - "file descriptor is associated with a socket, cannot seek", - ), - Stream(_) => syscall_error( - Errno::ESPIPE, - "lseek", - "file descriptor is associated with a stream, cannot seek", - ), - Pipe(_) => syscall_error( - Errno::ESPIPE, - "lseek", - "file descriptor is associated with a pipe, cannot seek", - ), - Epoll(_) => syscall_error( - Errno::ESPIPE, - "lseek", - "file descriptor is associated with an epollfd, cannot seek", - ), } } else { syscall_error(Errno::EBADF, "lseek", "invalid file descriptor") @@ -3058,58 +3146,136 @@ impl Cage { return dupfd; } - //------------------------------------CLOSE SYSCALL------------------------------------ - + /// ## ------------------CLOSE SYSCALL------------------ + /// ### Description + /// + /// The `close_syscall()` is used to close a file descriptor, so that it no + /// longer refers to any file and may be reused. This is an essential + /// operation for managing resources, as file descriptors are a limited + /// resource in any operating system. + /// + /// ### Function Arguments + /// + /// The `close_syscall()` receives one argument: + /// * `fd` - This argument refers to the file descriptor to be closed. + /// + /// ### Returns + /// + /// Upon successful completion of this call, it returns 0. Otherwise, it + /// returns error number set with the appropriate error code. + /// + /// ### Errors + /// + /// * The syscall returns errors which occur in its helper functions. + /// + /// ### Panics + /// + /// * This syscall indirectly panics when the helper function panics. + /// + /// For more detailed description of all the commands and return values, see + /// [close(2)](https://man7.org/linux/man-pages/man2/close.2.html) pub fn close_syscall(&self, fd: i32) -> i32 { - //check that the fd is valid + // Call the helper function which validates the result and deletes + // reference of the file descriptor. return Self::_close_helper(self, fd); } + /// ## ----------------- CLOSE HELPER INNER ------------------ + /// ### Description + /// + /// The `_close_helper_inner` function manages the cleanup and closing of + /// various types of file descriptors (files, directories, pipes, sockets) + /// by decrementing reference counts and removing inodes if necessary. + /// ### Function Arguments + /// + /// The function receives one argument: + /// * `fd` - This argument refers to the file descriptor to be closed. + /// + /// ### Returns + /// + /// Upon successful completion of this call, it returns 0. Otherwise, it + /// returns errors set with the appropriate error code. + /// + /// ### Errors + /// + /// * EBADF - The given file descriptor is invalid + /// * ENOEXEC - The Non-regular type file (dir/chardev) in file object table + /// + /// ### Panics + /// + /// * If the provided file descriptor is out of bounds, causing unwrap() to + /// panic + /// * When the file type filedescriptor contains a Socket as an inode. + /// + /// For more detailed description of all the commands and return values, see + /// [close(2)](https://man7.org/linux/man-pages/man2/close.2.html) pub fn _close_helper_inner(&self, fd: i32) -> i32 { + //BUG + //if the provided file descriptor is out of bounds, get_filedescriptor returns + // Err(), unwrapping on which produces a 'panic!' + //otherwise, file descriptor table entry is stored in 'checkedfd' let checkedfd = self.get_filedescriptor(fd).unwrap(); let mut unlocked_fd = checkedfd.write(); if let Some(filedesc_enum) = &mut *unlocked_fd { - //Decide how to proceed depending on the fd type. - //First we check in the file descriptor to handle sockets (no-op), sockets - // (clean the socket), and pipes (clean the pipe), and if it is a - // normal file descriptor we decrement the refcount to reflect + // We decide, how to proceed depending on the fd type. + // First we check in the file descriptor to handle stream / epoll (no-op), + // sockets (clean the socket), and pipes (clean the pipe), and if it is a + // normal file descriptor we decrement the reference count to reflect // one less reference to the file. match filedesc_enum { //if we are a socket, we dont change disk metadata - Stream(_) => {} - Epoll(_) => {} //Epoll closing not implemented yet + Stream(_) => {} // Streams don't require any additional cleanup + Epoll(_) => {} // TODO: Epoll closing not implemented yet Socket(ref mut socket_filedesc_obj) => { + // Retrieve the socket file descriptor object and get the write + // lock on the socket handle. let sock_tmp = socket_filedesc_obj.handle.clone(); let mut sockhandle = sock_tmp.write(); - // we need to do the following if UDS - if let Some(ref mut ui) = sockhandle.unix_info { - let inodenum = ui.inode; - if let Some(sendpipe) = ui.sendpipe.as_ref() { - sendpipe.decr_ref(O_WRONLY); - //last reference, lets remove it - if sendpipe.is_pipe_closed() { - ui.sendpipe = None; + // we need to do the following if UDS (Unix Domain Socket) + // AF_UNIX represents the Unix domain sockets. + let socket_type = sockhandle.domain; + if socket_type == AF_UNIX { + if let Some(ref mut ui) = sockhandle.unix_info { + let inodenum = ui.inode; + // Decrement the reference count for the send pipe if it exists. If the + // reference count drops to zero, remove the send pipe. + if let Some(sendpipe) = ui.sendpipe.as_ref() { + sendpipe.decr_ref(O_WRONLY); + //last reference, lets remove it + if sendpipe.is_pipe_closed() { + ui.sendpipe = None; + } } - } - if let Some(receivepipe) = ui.receivepipe.as_ref() { - receivepipe.decr_ref(O_RDONLY); - //last reference, lets remove it - if receivepipe.is_pipe_closed() { - ui.receivepipe = None; + // Decrement the reference count for the receive pipe if it exists. If + // the reference count drops to zero, remove + // the receive pipe. + if let Some(receivepipe) = ui.receivepipe.as_ref() { + receivepipe.decr_ref(O_RDONLY); + //last reference, lets remove it + if receivepipe.is_pipe_closed() { + ui.receivepipe = None; + } } - } - let mut inodeobj = FS_METADATA.inodetable.get_mut(&ui.inode).unwrap(); - if let Inode::Socket(ref mut sock) = *inodeobj { - sock.refcount -= 1; - if sock.refcount == 0 { - if sock.linkcount == 0 { + // Retrieve the inode object for the socket and decrement its reference + // count. If both the reference count and + // link count are zero, the socket is no longer needed. + let mut inodeobj = FS_METADATA.inodetable.get_mut(&ui.inode).unwrap(); + if let Inode::Socket(ref mut sock) = *inodeobj { + sock.refcount -= 1; + if sock.refcount == 0 && sock.linkcount == 0 { + // Remove the socket from the inode table and the domain + // socket paths if it is no longer needed. drop(inodeobj); + // Get the entire path of the socket which is used for removing + // it from the metadata file. let path = normpath( convpath(sockhandle.localaddr.unwrap().path()), self, ); + // Remove the reference of the inode from the inodetable FS_METADATA.inodetable.remove(&inodenum); + // Remove any domain socket paths associated with the socket NET_METADATA.domsock_paths.remove(&path); } } @@ -3117,11 +3283,12 @@ impl Cage { } } Pipe(ref pipe_filedesc_obj) => { - // lets decrease the pipe objects internal ref count for the corresponding end - // depending on what flags are set + // Decrease the pipe objects internal ref count for the corresponding end + // depending on what flags are set (O_RDONLY or O_WRONLY). pipe_filedesc_obj.pipe.decr_ref(pipe_filedesc_obj.flags); } File(ref normalfile_filedesc_obj) => { + // Retrieve the inode object for the file. let inodenum = normalfile_filedesc_obj.inode; let mut inodeobj = FS_METADATA.inodetable.get_mut(&inodenum).unwrap(); @@ -3129,20 +3296,31 @@ impl Cage { Inode::File(ref mut normalfile_inode_obj) => { normalfile_inode_obj.refcount -= 1; - //if it's not a reg file, then we have nothing to close - //Inode::File is a regular file by default + // Check if it's not a regular file, then we have nothing to close + // Inode::File is a regular file by default + // Reference count represents the number of active file descriptors + // pointing to the file. if normalfile_inode_obj.refcount == 0 { + // FileObjectTable stores the entries of the currently opened files + // in the system, so if there are no + // active file descriptors pointing to the + // file, we delete them from the table. FILEOBJECTTABLE .remove(&inodenum) .unwrap() .1 .close() .unwrap(); + // Link count as 0 represents that there are no hard links present + // for the file, so we need to remove it from the filesystem. if normalfile_inode_obj.linkcount == 0 { drop(inodeobj); - //removing the file from the entire filesystem (interface, + // removing the file from the entire filesystem (interface, // metadata, and object table) FS_METADATA.inodetable.remove(&inodenum); + // FILEDATAPREFIX represents the common prefix of the name + // of the file which combined with the inode number represents + // a unique entity. It stores the data of the inode object. let sysfilename = format!("{}{}", FILEDATAPREFIX, inodenum); interface::removefile(sysfilename).unwrap(); log_metadata(&FS_METADATA, inodenum); @@ -3154,17 +3332,24 @@ impl Cage { Inode::Dir(ref mut dir_inode_obj) => { dir_inode_obj.refcount -= 1; - //if it's not a reg file, then we have nothing to close + // File object table only contains references for the regular + // type files, and since "Directory" is not a regular file type, + // it should not exist in the table. Return an error if the + // directory exists in the file object table. match FILEOBJECTTABLE.get(&inodenum) { Some(_) => { return syscall_error( Errno::ENOEXEC, "close or dup", - "Non-regular file in file object table", + "Non-regular file (dir) in file object table", ); } + // Continue if the object table doesn't contain the inode. None => {} } + // When the link count is 2 and the reference count is 0, it means + // the directory is empty and no processes are using it. This allows + // the directory to be safely removed from the file system. if dir_inode_obj.linkcount == 2 && dir_inode_obj.refcount == 0 { //The reference to the inode has to be dropped to avoid //deadlocking because the remove() method will need to @@ -3180,16 +3365,18 @@ impl Cage { } Inode::CharDev(ref mut char_inode_obj) => { char_inode_obj.refcount -= 1; - - //if it's not a reg file, then we have nothing to close + // Since "CharDev" is not a regular file type, it should not + // exist in the file object table. Return an error if the + // chardev inode exists in the file object table. match FILEOBJECTTABLE.get(&inodenum) { Some(_) => { return syscall_error( Errno::ENOEXEC, "close or dup", - "Non-regular file in file object table", + "Non-regular file (chardev) in file object table", ); } + // Continue if the object table doesn't contain the inode. None => {} } if char_inode_obj.linkcount == 0 && char_inode_obj.refcount == 0 { @@ -3201,27 +3388,72 @@ impl Cage { } log_metadata(&FS_METADATA, inodenum); } + // A Sanity check is added to make sure that there is no such case when the + // fd type is "File" and the inode type is "Socket". This state is ideally + // not possible, so we panic in such cases. Inode::Socket(_) => { panic!("close(): Socket inode found on a filedesc fd.") } } } } + // If everything is successful, we return 0 0 } else { return syscall_error(Errno::EBADF, "close", "invalid file descriptor"); } } + /// ## ------------------CLOSE HELPER ------------------ + /// ### Description + /// + /// The `_close_helper` function calls `_close_helper_inner` to handle + /// the specifics of closing the file descriptor (like decrementing + /// reference counts, handling special cases for sockets, pipes, etc.). + /// Once `_close_helper_inner` has successfully done its job (indicated + /// by returning a non-negative value), `_close_helper` removes the file + /// descriptor from the file descriptor table. This ensures that the file + /// descriptor is fully closed and no longer tracked by the system. + /// + /// ### Function Arguments + /// + /// The `_close_helper()` receives one argument: + /// * `fd` - This argument refers to the file descriptor to be closed. + /// + /// ### Returns + /// + /// Upon successful completion of this call, it returns 0. Otherwise, it + /// returns an error number with the appropriate error code. + /// + /// ### Errors + /// + /// * There are no explicit errors returned by this function. The errors + /// thrown by `_close_helper_inner` function are returned. + /// + /// ### Panics + /// + /// * If the provided file descriptor is out of bounds, causing unwrap() to + /// panic. pub fn _close_helper(&self, fd: i32) -> i32 { let inner_result = self._close_helper_inner(fd); + // Return value of less than 0 indicates that there was some error + // closing the fd, so we return the error. if inner_result < 0 { return inner_result; } - //removing descriptor from fd table + // Once we know that the closing of the fd was successful, + // we remove the file descriptor from the fd table and free the space. + // + // Bug: + // if the provided file descriptor is out of bounds, get_filedescriptor returns + // Err(), unwrapping on which produces a 'panic!' + //otherwise, file descriptor table entry is stored in 'checkedfd' let checkedfd = self.get_filedescriptor(fd).unwrap(); let mut unlocked_fd = checkedfd.write(); + // This operation effectively removes the file descriptor from the fd table + // by removing its reference from the variable and then not using it further, + // allowing it to be dropped if unlocked_fd.is_some() { let _discarded_fd = unlocked_fd.take(); } diff --git a/src/tests/fs_tests.rs b/src/tests/fs_tests.rs index 193bcf0a6..ed99bf4ae 100644 --- a/src/tests/fs_tests.rs +++ b/src/tests/fs_tests.rs @@ -4066,4 +4066,332 @@ pub mod fs_tests { lindrustfinalize(); } + + #[test] + pub fn ut_lind_fs_lseek_on_file() { + // 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); + + // Test to create a file and check if seeking to a new location is possible. + let fd = cage.open_syscall("/test_file", O_CREAT | O_WRONLY, S_IRWXA); + assert!(fd >= 0); + + // Attempt to seek within the file and check if it succeeds + assert_eq!(cage.lseek_syscall(fd, 10, SEEK_SET), 10); + + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); + lindrustfinalize(); + } + + #[test] + pub fn ut_lind_fs_lseek_on_directory() { + // 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); + + // Create a directory and try to seek within it. + let path = "/test_dir"; + assert_eq!(cage.mkdir_syscall(path, S_IRWXA), 0); + let fd = cage.open_syscall(path, O_RDONLY, S_IRWXA); + assert!(fd >= 0); + + // Attempt to seek within the directory and check if it succeeds + assert_eq!(cage.lseek_syscall(fd, 1, SEEK_SET), 1); + + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); + lindrustfinalize(); + } + + #[test] + pub fn ut_lind_fs_lseek_invalid_whence() { + // 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); + + // Test to create a file and check for invalid `whence` value + let fd = cage.open_syscall("/test_file", O_CREAT | O_RDWR, S_IRWXA); + assert!(fd >= 0); + + // Attempt to seek with an invalid `whence` value and check if it returns an + // error + assert_eq!( + cage.lseek_syscall(fd, 10, 999), // Invalid whence value + -(Errno::EINVAL as i32) + ); + + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); + lindrustfinalize(); + } + + #[test] + pub fn ut_lind_fs_lseek_beyond_file_size() { + // 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); + + // Test to create a file and seek beyond its size + let fd = cage.open_syscall("/test_file", O_CREAT | O_RDWR, S_IRWXA); + assert!(fd >= 0); + + // Write sample data to the file. + assert_eq!(cage.write_syscall(fd, str2cbuf("hello"), 5), 5); + + // Seek beyond the end of the file and verify if it succeeds + assert_eq!( + cage.lseek_syscall(fd, 10, SEEK_END), + 15 // 5 (file size) + 10 (offset) + ); + + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); + lindrustfinalize(); + } + + #[test] + pub fn ut_lind_fs_lseek_before_start_of_file() { + // 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); + + // Test to create a file and attempt to seek before the start of the file + let fd = cage.open_syscall("/test_file", O_CREAT | O_RDWR, S_IRWXA); + assert!(fd >= 0); + + // Attempt to seek to a negative offset and check if it returns an error + // using "SEEK_SET" whence, where we are explicitly setting the file + // offset to -10 value. + assert_eq!( + cage.lseek_syscall(fd, -10, SEEK_SET), + -(Errno::EINVAL as i32) + ); + + // Attempt to seek to a negative offset and check if it returns an error + // using "SEEK_CUR" whence, where current position of the file is 0, + // as it's empty initially, and we are adding -10 to the offset. + assert_eq!( + cage.lseek_syscall(fd, -10, SEEK_CUR), + -(Errno::EINVAL as i32) + ); + + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); + lindrustfinalize(); + } + + #[test] + pub fn ut_lind_fs_lseek_on_pipe() { + // 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); + + // Create a pipe and attempt to seek within it + let mut pipe_fds = PipeArray::default(); + assert_eq!(cage.pipe_syscall(&mut pipe_fds), 0); + let read_fd = pipe_fds.readfd; + + // Attempt to seek within the pipe and check if it returns an error + assert_eq!( + cage.lseek_syscall(read_fd, 10, SEEK_SET), + -(Errno::ESPIPE as i32) + ); + + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); + lindrustfinalize(); + } + + #[test] + pub fn ut_lind_fs_lseek_on_chardev() { + // 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); + + // Attempt to seek within a character device file + let path = "/dev/null"; + let fd = cage.open_syscall(path, O_RDWR, S_IRWXA); + + // Seek within the character device and check if it returns 0 (no operation) + assert_eq!(cage.lseek_syscall(fd, 10, SEEK_SET), 0); + + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); + lindrustfinalize(); + } + + #[test] + pub fn ut_lind_fs_lseek_on_epoll() { + // 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); + + // Create an Epoll and try to seek from it. + let epfd = cage.epoll_create_syscall(1); + assert!(epfd > 0); + + // Attempt to seek from the epoll and check if it returns an error + assert_eq!( + cage.lseek_syscall(epfd, 10, SEEK_SET), + -(Errno::ESPIPE as i32) + ); + + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); + lindrustfinalize(); + } + + #[test] + pub fn ut_lind_fs_close_regular_file() { + // 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); + + // Create and open a regular file, then close it. + let fd = cage.open_syscall("/test_file", O_CREAT | O_RDWR, S_IRWXA); + assert!(fd >= 0); + + // Write sample data to the file. + assert_eq!(cage.write_syscall(fd, str2cbuf("hello"), 5), 5); + + // Close the file descriptor, which should succeed. + assert_eq!(cage.close_syscall(fd), 0); + + // Attempt to close the file descriptor again to ensure it's already closed. + // Expect an error for "Invalid File Descriptor". + assert_eq!(cage.close_syscall(fd), -(Errno::EBADF as i32)); + + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); + lindrustfinalize(); + } + + #[test] + pub fn ut_lind_fs_close_directory() { + // 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); + + // Create a directory and open it. + let path = "/test_dir"; + assert_eq!(cage.mkdir_syscall(path, S_IRWXA), 0); + let fd = cage.open_syscall(path, O_RDONLY, S_IRWXA); + assert!(fd >= 0); + + // Close the directory file descriptor, which should succeed. + assert_eq!(cage.close_syscall(fd), 0); + + // Attempt to close the file descriptor again to ensure it's already closed. + // Expect an error for "Invalid File Descriptor". + assert_eq!(cage.close_syscall(fd), -(Errno::EBADF as i32)); + + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); + lindrustfinalize(); + } + + #[test] + pub fn ut_lind_fs_close_socket() { + // 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); + + // Create a socket pair. + let mut socketpair = interface::SockPair::default(); + assert_eq!( + Cage::socketpair_syscall(cage.clone(), AF_UNIX, SOCK_STREAM, 0, &mut socketpair), + 0 + ); + + // Close both the socket file descriptors, which should succeed. + assert_eq!(cage.close_syscall(socketpair.sock1), 0); + assert_eq!(cage.close_syscall(socketpair.sock2), 0); + + // Attempt to close the file descriptors again to ensure they are already + // closed. Expect an error for "Invalid File Descriptor". + assert_eq!(cage.close_syscall(socketpair.sock1), -(Errno::EBADF as i32)); + assert_eq!(cage.close_syscall(socketpair.sock2), -(Errno::EBADF as i32)); + + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); + lindrustfinalize(); + } + + #[test] + pub fn ut_lind_fs_close_pipe() { + // 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); + + // Create a pipe. + let mut pipe_fds = PipeArray::default(); + assert_eq!(cage.pipe_syscall(&mut pipe_fds), 0); + let read_fd = pipe_fds.readfd; + let write_fd = pipe_fds.writefd; + + // Write data to the pipe + let write_data = "Testing"; + assert_eq!( + cage.write_syscall(write_fd, write_data.as_ptr(), write_data.len()), + write_data.len() as i32 + ); + + // Read the data from the pipe. + let mut buf = sizecbuf(7); + assert_eq!( + cage.read_syscall(read_fd, buf.as_mut_ptr(), buf.len()), + write_data.len() as i32 + ); + assert_eq!(cbuf2str(&buf), write_data); + + // Close the pipe file descriptors, which should succeed. + assert_eq!(cage.close_syscall(read_fd), 0); + assert_eq!(cage.close_syscall(write_fd), 0); + + // Attempt to close the file descriptor again to ensure they are already closed. + // Expect an error for "Invalid File Descriptor". + assert_eq!(cage.close_syscall(read_fd), -(Errno::EBADF as i32)); + assert_eq!(cage.close_syscall(write_fd), -(Errno::EBADF as i32)); + + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); + lindrustfinalize(); + } + + #[test] + pub fn ut_lind_fs_close_chardev() { + // 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); + + // Open a character device file. + let fd = cage.open_syscall("/dev/zero", O_RDWR, S_IRWXA); + assert!(fd >= 0); + + // Close the character device file descriptor, which should succeed. + assert_eq!(cage.close_syscall(fd), 0); + + // Attempt to close the file descriptor again to ensure it's already closed. + // Expect an error for "Invalid File Descriptor". + assert_eq!(cage.close_syscall(fd), -(Errno::EBADF as i32)); + + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); + lindrustfinalize(); + } } From 18b7895c698872ed5501bc8289e673a361eb207d Mon Sep 17 00:00:00 2001 From: Qianxi Chen <53324229+qianxichen233@users.noreply.github.com> Date: Mon, 29 Jul 2024 23:41:16 +0800 Subject: [PATCH 2/5] test and comment for sockopt syscalls (#315) * added some comments for sockopts * finish sockopt test and comment * resolved comments --------- Co-authored-by: lind --- src/safeposix/syscalls/net_calls.rs | 242 ++++++++++++++++++++++-- src/safeposix/syscalls/net_constants.rs | 7 + src/tests/networking_tests.rs | 182 +++++++++++++++++- 3 files changed, 407 insertions(+), 24 deletions(-) diff --git a/src/safeposix/syscalls/net_calls.rs b/src/safeposix/syscalls/net_calls.rs index 50cfe52fa..6677e7bae 100644 --- a/src/safeposix/syscalls/net_calls.rs +++ b/src/safeposix/syscalls/net_calls.rs @@ -3173,18 +3173,97 @@ impl Cage { return 0; } + /// ## ------------------GETSOCKOPT SYSCALL------------------ + /// ### Description + /// "getsockopt_syscall()" retrieves the option specified by the optname + /// argument at the protocol level specified by the level argument for + /// the socket associated with the file descriptor specified by the fd + /// argument, and stores the result in the optval argument. + /// + /// ### Function Arguments + /// The `getsockopt_syscall()` receives four arguments: + /// * `fd` - The file descriptor to retrieve the socket option. + /// * `level` - the protocol level at which the option resides. To get + /// options at the socket level, specify the level argument as SOL_SOCKET. + /// To get options at other levels, supply the appropriate level + /// identifier for the protocol controlling the option. + /// * `optname` - The name of the option + /// * `optval` - The buffer to hold the return value + /// + /// ### Returns + /// Upon successful completion, getsockopt_syscall() shall return 0. + /// + /// ### Errors + /// * EBADF - The socket argument is not a valid file descriptor. + /// * ENOPROTOOPT - The option is unknown at the level indicated. + /// * ENOTSOCK - The file descriptor sockfd does not refer to a socket. + /// + /// ### Panics + /// No panic is expected from this syscall + /// + /// more details at https://man7.org/linux/man-pages/man2/getsockopt.2.html + /// more details for avaliable socket options at + /// https://man7.org/linux/man-pages/man7/socket.7.html + /// https://man7.org/linux/man-pages/man7/tcp.7.html pub fn getsockopt_syscall(&self, fd: i32, level: i32, optname: i32, optval: &mut i32) -> i32 { + // The current sockopt syscalls have issues storing the option values. Our + // approach uses the optname as the bit position of the option, meaning + // the i-th bit of the option corresponds to the i-th optname. This + // causes problems because we use a 32-bit integer to store both the + // socket option and the TCP option, leading to overflow if any optname + // is larger than 32. + // + // Linux handles this differently. For TCP options, the option values are not + // stored centrally; each option is handled separately and may + // correspond to multiple flags being turned on or off. For socket + // options, Linux stores some options in a single location (sk_flags) + // using a similar approach to ours. However, Linux uses a separate + // layer of internal flags specifically for those stored in sk_flags. These + // internal flag values are of enum type and are sequential, indicating + // the bit position of the option. Linux creates a mapping from + // user-facing socket optnames (e.g., SO_KEEPALIVE) to internal flags. + // This ensures that optnames not meant for sk_flags do not affect the + // sequential order of sk_flags bits, and those that should be stored in + // sk_flags are grouped efficiently. This allows Linux to support more + // than 32 socket options while correctly storing some boolean socket + // options in sk_flags, a 32-bit integer. + // + // other issues include: + // 1. many of the options such as SO_SNDBUF, SO_RCVBUF, though are stored in + // sockhandle, never get used anywhere + // 2. when we set the socket options before bind/connect, these options will not + // be set with libc setsockopt since innersocket hasn’t been created yet. But + // when later the innersocket is created, we did not set these options to + // innersocket + // 3. the optval argument is not supposed to be an integer type. Optval for some + // optname is a struct. + + // first let's check the fd range + if fd < 0 || fd >= MAXFD { + return syscall_error( + Errno::EBADF, + "getsockopt", + "provided fd is not a valid file descriptor", + ); + } + + // try to get the file descriptor object let checkedfd = self.get_filedescriptor(fd).unwrap(); let mut unlocked_fd = checkedfd.write(); if let Some(filedesc_enum) = &mut *unlocked_fd { if let Socket(ref mut sockfdobj) = filedesc_enum { + // we store the options inside a 32-bit integer + // where i-th bits corresponds to i-th option let optbit = 1 << optname; + // get the write lock of the socket handler let sock_tmp = sockfdobj.handle.clone(); let mut sockhandle = sock_tmp.write(); match level { + // a few UDP options are avaliable in Linux + // though we do not support them for now SOL_UDP => { return syscall_error( - Errno::EOPNOTSUPP, + Errno::ENOPROTOOPT, "getsockopt", "UDP is not supported for getsockopt", ); @@ -3192,17 +3271,30 @@ impl Cage { SOL_TCP => { // Checking the tcp_options here // Currently only support TCP_NODELAY option for SOL_TCP + + // TCP_NODELAY: If set, disable the Nagle algorithm. This means that + // segments are always sent as soon as possible, even if + // there is only a small amount of data. When not set, data + // is buffered until there is a sufficient amount to send + // out, thereby avoiding the frequent sending of small + // packets, which results in poor utilization of the network. + // This option is overridden by TCP_CORK; however, setting + // this option forces an explicit flush of pending output, + // even if TCP_CORK is currently set. if optname == TCP_NODELAY { let optbit = 1 << optname; if optbit & sockhandle.tcp_options == optbit { + // if the bit is set, set optval to 1 *optval = 1; } else { + // otherwise, set optval to 0 *optval = 0; } return 0; } + // other TCP options are not supported yet return syscall_error( - Errno::EOPNOTSUPP, + Errno::ENOPROTOOPT, "getsockopt", "TCP options not remembered by getsockopt", ); @@ -3210,46 +3302,85 @@ impl Cage { SOL_SOCKET => { // checking the socket_options here match optname { - //indicate whether we are accepting connections or not in the moment + // indicate whether we are accepting connections or not in the moment SO_ACCEPTCONN => { if sockhandle.state == ConnState::LISTEN { + // if in LISTEN state, set return value to 1 *optval = 1; } else { + // otherwise, set return value to 0 *optval = 0; } } - //if the option is a stored binary option, just return it... + // these options are stored inside the socket_options + // so we just retrieve it and set to optval + // BUG: SO_LINGER is not supposed to be a boolean option + + // SO_LINGER: When enabled, a close or shutdown will not return + // until all queued messages for the socket have been + // successfully sent or the linger timeout has been reached. + // Otherwise, the call returns immediately and the closing is + // done in the background. When the socket is closed as part + // of exit, it always lingers in the background. + + // SO_KEEPALIVE: Enable sending of keep-alive messages on connection- + // oriented sockets. Expects an integer boolean flag. + + // SO_SNDLOWAT/SO_RCVLOWAT: Specify the minimum number of bytes in the + // buffer until the socket layer will pass + // the data to the protocol (SO_SNDLOWAT) or + // the user on receiving (SO_RCVLOWAT). + + // SO_REUSEPORT: Permits multiple AF_INET or AF_INET6 sockets to be + // bound to an identical socket address. + + // SO_REUSEADDR: Indicates that the rules used in validating addresses + // supplied in a bind call should allow reuse of local addresses. SO_LINGER | SO_KEEPALIVE | SO_SNDLOWAT | SO_RCVLOWAT | SO_REUSEPORT | SO_REUSEADDR => { if sockhandle.socket_options & optbit == optbit { + // if the bit is set, set optval to 1 *optval = 1; } else { + // otherwise, set optval to 0 *optval = 0; } } - //handling the ignored buffer settings: + // sndbuf, rcvbuf, socktype are stored in a dedicated field + // so retrieve it directly and set the optval to it + + // SO_SNDBUF: Sets or gets the maximum socket send buffer in bytes. + // SO_RCVBUF: Sets or gets the maximum socket receive buffer in bytes. SO_SNDBUF => { *optval = sockhandle.sndbuf; } SO_RCVBUF => { *optval = sockhandle.rcvbuf; } - //returning the type if asked + // SO_TYPE: Gets the socket type as an integer SO_TYPE => { *optval = sockhandle.socktype; } - //should always be true + // If SO_OOBINLINE is enabled, out-of-band data is directly + // placed into the receive data stream. Otherwise, out-of- + // band data is passed only when the MSG_OOB flag is set + // during receiving. + // currently we do not support changing this value + // so it should always be 1 SO_OOBINLINE => { *optval = 1; } + // Get and clear the pending socket error. This socket + // option is read-only. SO_ERROR => { let tmp = sockhandle.errno; sockhandle.errno = 0; *optval = tmp; } _ => { + // we do not support other options currently return syscall_error( - Errno::EOPNOTSUPP, + Errno::ENOPROTOOPT, "getsockopt", "unknown optname passed into syscall", ); @@ -3257,6 +3388,7 @@ impl Cage { } } _ => { + // we do not support other levels yet return syscall_error( Errno::EOPNOTSUPP, "getsockopt", @@ -3265,6 +3397,7 @@ impl Cage { } } } else { + // the file descriptor is not socket fd return syscall_error( Errno::ENOTSOCK, "getsockopt", @@ -3272,6 +3405,7 @@ impl Cage { ); } } else { + // the file descriptor does not exist return syscall_error( Errno::EBADF, "getsockopt", @@ -3281,14 +3415,64 @@ impl Cage { return 0; } + /// ## ------------------SETSOCKOPT SYSCALL------------------ + /// ### Description + /// "setsockopt_syscall()" sets a socket option. It configures the option + /// specified by the optname argument, at the protocol level specified + /// by the level argument, to the value pointed to by the optval argument. + /// This is done for the socket associated with the file descriptor provided + /// in the fd argument. + /// + /// + /// ### Function Arguments + /// The `setsockopt_syscall()` receives four arguments: + /// * `fd` - The file descriptor to retrieve the socket option. + /// * `level` - the protocol level at which the option resides. To set + /// options at the socket level, specify the level argument as SOL_SOCKET. + /// To set options at other levels, supply the appropriate level + /// identifier for the protocol controlling the option. + /// * `optname` - The name of the option + /// * `optval` - The value of the option + /// + /// ### Returns + /// Upon successful completion, setsockopt_syscall() shall return 0. + /// + /// ### Errors + /// * EBADF - The socket argument is not a valid file descriptor. + /// * EINVAL - The specified option is invalid at the specified socket level + /// or the socket has been shut down. + /// * EISCONN - The socket is already connected, and a specified option + /// cannot be set while the socket is connected. + /// * ENOPROTOOPT - The option is unknown at the level indicated. + /// * ENOTSOCK - The file descriptor sockfd does not refer to a socket. + /// + /// ### Panics + /// No panic is expected from this syscall + /// + /// more details at https://man7.org/linux/man-pages/man3/setsockopt.3p.html pub fn setsockopt_syscall(&self, fd: i32, level: i32, optname: i32, optval: i32) -> i32 { + // there are some issues with sockopt syscalls. See comment at + // getsockopt_syscall for more detail + + // first let's check the fd range + if fd < 0 || fd >= MAXFD { + return syscall_error( + Errno::EBADF, + "setsockopt", + "provided fd is not a valid file descriptor", + ); + } + + // get the file descriptor object let checkedfd = self.get_filedescriptor(fd).unwrap(); let mut unlocked_fd = checkedfd.write(); if let Some(filedesc_enum) = &mut *unlocked_fd { if let Socket(ref mut sockfdobj) = filedesc_enum { - //checking that we recieved SOL_SOCKET + // for the explanation of each socket options, check + // getsockopt_syscall at corresponding location match level { SOL_UDP => { + // we do not support SOL_UDP return syscall_error( Errno::EOPNOTSUPP, "setsockopt", @@ -3299,22 +3483,28 @@ impl Cage { // Here we check and set tcp_options // Currently only support TCP_NODELAY for SOL_TCP if optname == TCP_NODELAY { + // we store this flag in tcp_options at the corresponding bit let optbit = 1 << optname; let sock_tmp = sockfdobj.handle.clone(); let mut sockhandle = sock_tmp.write(); let mut newoptions = sockhandle.tcp_options; - //now let's set this if we were told to + // now let's set this if we were told to + // optval should always be 1 or 0. if optval != 0 { - //optval should always be 1 or 0. + // set the bit newoptions |= optbit; } else { + // clear the bit newoptions &= !optbit; } + // if the tcp option changed, we need to call underlining + // setsockopt on rawfd to actually set the option with libc setsockopt if newoptions != sockhandle.tcp_options { if let Some(sock) = sockhandle.innersocket.as_ref() { let sockret = sock.setsockopt(SOL_TCP, optname, optval); if sockret < 0 { + // error returned from libc setsockopt match Errno::from_discriminant(interface::get_errno()) { Ok(i) => { return syscall_error( @@ -3330,9 +3520,11 @@ impl Cage { } } } + // store the new options sockhandle.tcp_options = newoptions; return 0; } + // we do not support other TCP options yet return syscall_error( Errno::EOPNOTSUPP, "setsockopt", @@ -3341,12 +3533,14 @@ impl Cage { } SOL_SOCKET => { // Here we check and set socket_options + // we store this flag in socket_options at the corresponding bit let optbit = 1 << optname; let sock_tmp = sockfdobj.handle.clone(); let mut sockhandle = sock_tmp.write(); match optname { SO_ACCEPTCONN | SO_TYPE | SO_SNDLOWAT | SO_RCVLOWAT => { + // these socket options are read-only and cannot be manually set let error_string = format!("Cannot set option using setsockopt. {}", optname); return syscall_error( @@ -3356,30 +3550,41 @@ impl Cage { ); } SO_LINGER | SO_KEEPALIVE => { + // these socket options are stored inside socket_options + // so we just modify it in socket_options + // optval should always be 1 or 0. if optval == 0 { + // clear the bit sockhandle.socket_options &= !optbit; } else { - //optval should always be 1 or 0. + // set the bit sockhandle.socket_options |= optbit; } + // BUG: we did not pass these options to libc setsockopt return 0; } SO_REUSEPORT | SO_REUSEADDR => { let mut newoptions = sockhandle.socket_options; - //now let's set this if we were told to + // now let's set this if we were told to + // optval should always be 1 or 0. if optval != 0 { - //optval should always be 1 or 0. + // set the bit newoptions |= optbit; } else { + // clear the bit newoptions &= !optbit; } + // if the socket option changed, we need to call underlining + // setsockopt on rawfd to actually set the option with libc + // setsockopt if newoptions != sockhandle.socket_options { if let Some(sock) = sockhandle.innersocket.as_ref() { let sockret = sock.setsockopt(SOL_SOCKET, optname, optval); if sockret < 0 { + // error from libc setsockopt match Errno::from_discriminant(interface::get_errno()) { Ok(i) => { return syscall_error( @@ -3396,10 +3601,13 @@ impl Cage { } } + // set the new options sockhandle.socket_options = newoptions; return 0; } + // sndbuf and rcvbuf are stored in a dedicated field + // so we just set it SO_SNDBUF => { sockhandle.sndbuf = optval; return 0; @@ -3408,7 +3616,7 @@ impl Cage { sockhandle.rcvbuf = optval; return 0; } - //should always be one -- can only handle it being 1 + // we do not support changing this option yet SO_OOBINLINE => { if optval != 1 { return syscall_error( @@ -3419,6 +3627,7 @@ impl Cage { } return 0; } + // other options are either not supported or invalid _ => { return syscall_error( Errno::EOPNOTSUPP, @@ -3429,6 +3638,7 @@ impl Cage { } } _ => { + // invalid level return syscall_error( Errno::EOPNOTSUPP, "getsockopt", @@ -3437,6 +3647,7 @@ impl Cage { } } } else { + // the fd is not a socket return syscall_error( Errno::ENOTSOCK, "getsockopt", @@ -3444,6 +3655,7 @@ impl Cage { ); } } else { + // the fd is not a valid file descriptor return syscall_error( Errno::EBADF, "getsockopt", diff --git a/src/safeposix/syscalls/net_constants.rs b/src/safeposix/syscalls/net_constants.rs index 6f84cb677..0b91cf304 100644 --- a/src/safeposix/syscalls/net_constants.rs +++ b/src/safeposix/syscalls/net_constants.rs @@ -334,6 +334,13 @@ pub const SO_ACCEPTCONN: i32 = 30; pub const SOL_TCP: i32 = IPPROTO_TCP; pub const SOL_UDP: i32 = IPPROTO_UDP; +// some TCP flags below are not part of Linux standards +// for example, TCP_NOPUSH is the flag used in FreeBSD/MacOS +// while the actual flag in Linux that serves the similar purpose +// is TCP_CORK +// Besides, some other flags are also not found in Linux man page: +// TCP_KEEPALIVE, TCP_CONNECTIONTIMEOUT, PERSIST_TIMEOUT, TCP_RXT_CONNDROPTIME +// and TCP_RXT_FINDROP pub const TCP_NODELAY: i32 = 0x01; // don't delay send to coalesce packets pub const TCP_MAXSEG: i32 = 0x02; // set maximum segment size pub const TCP_NOPUSH: i32 = 0x04; // don't push last block of write diff --git a/src/tests/networking_tests.rs b/src/tests/networking_tests.rs index 32b2fe518..cdcb9d5dd 100644 --- a/src/tests/networking_tests.rs +++ b/src/tests/networking_tests.rs @@ -3403,9 +3403,165 @@ pub mod net_tests { lindrustfinalize(); } + #[test] + #[ignore] + pub fn ut_lind_net_sockopt_bad_input_optname() { + // this test is used for testing invalid optname that is + // large enough to overflow to bitwise shift + // would fail currently + + // 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 sockfd = cage.socket_syscall(AF_INET, SOCK_STREAM, 0); + let mut optstore = 0; + + assert_eq!( + cage.getsockopt_syscall(sockfd, SOL_TCP, 100, &mut optstore), + -(Errno::ENOPROTOOPT as i32) + ); + assert_eq!( + cage.getsockopt_syscall(sockfd, SOL_SOCKET, 100, &mut optstore), + -(Errno::ENOPROTOOPT as i32) + ); + + assert_eq!( + cage.setsockopt_syscall(sockfd, SOL_TCP, 100, 0), + -(Errno::EOPNOTSUPP as i32) + ); + assert_eq!( + cage.setsockopt_syscall(sockfd, SOL_SOCKET, 100, 0), + -(Errno::EOPNOTSUPP as i32) + ); + + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); + lindrustfinalize(); + } + + #[test] + pub fn ut_lind_net_getsockopt_bad_input() { + // this test is used for testing getsockopt_syscall with error input + + // 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 filefd = + cage.open_syscall("/netgetsockopttest.txt", O_CREAT | O_EXCL | O_RDWR, S_IRWXA); + let sockfd = cage.socket_syscall(AF_INET, SOCK_STREAM, 0); + + let mut optstore = -12; + + // unexist file descriptor + assert_eq!( + cage.getsockopt_syscall(10, SOL_SOCKET, SO_REUSEPORT, &mut optstore), + -(Errno::EBADF as i32) + ); + + // out of range file descriptor + assert_eq!( + cage.getsockopt_syscall(-1, SOL_SOCKET, SO_REUSEPORT, &mut optstore), + -(Errno::EBADF as i32) + ); + + // fd that is not socket + assert_eq!( + cage.getsockopt_syscall(filefd, SOL_SOCKET, SO_REUSEPORT, &mut optstore), + -(Errno::ENOTSOCK as i32) + ); + + // invalid level argument + assert_eq!( + cage.getsockopt_syscall(sockfd, 30, SO_REUSEPORT, &mut optstore), + -(Errno::EOPNOTSUPP as i32) + ); + + // unsupported level argument + assert_eq!( + cage.getsockopt_syscall(sockfd, SOL_UDP, SO_REUSEPORT, &mut optstore), + -(Errno::ENOPROTOOPT as i32) + ); + + // invalid optname argument + assert_eq!( + cage.getsockopt_syscall(sockfd, SOL_SOCKET, 20, &mut optstore), + -(Errno::ENOPROTOOPT as i32) + ); + assert_eq!( + cage.getsockopt_syscall(sockfd, SOL_TCP, 20, &mut optstore), + -(Errno::ENOPROTOOPT as i32) + ); + + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); + lindrustfinalize(); + } + + #[test] + pub fn ut_lind_net_setsockopt_bad_input() { + // this test is used for testing setsockopt_syscall with error input + + // 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 filefd = + cage.open_syscall("/netsetsockopttest.txt", O_CREAT | O_EXCL | O_RDWR, S_IRWXA); + let sockfd = cage.socket_syscall(AF_INET, SOCK_STREAM, 0); + + // unexist file descriptor + assert_eq!( + cage.setsockopt_syscall(10, SOL_SOCKET, SO_REUSEPORT, 0), + -(Errno::EBADF as i32) + ); + + // out of range file descriptor + assert_eq!( + cage.setsockopt_syscall(-1, SOL_SOCKET, SO_REUSEPORT, 0), + -(Errno::EBADF as i32) + ); + + // fd that is not socket + assert_eq!( + cage.setsockopt_syscall(filefd, SOL_SOCKET, SO_REUSEPORT, 0), + -(Errno::ENOTSOCK as i32) + ); + + // invalid level argument + assert_eq!( + cage.setsockopt_syscall(sockfd, 30, SO_REUSEPORT, 0), + -(Errno::EOPNOTSUPP as i32) + ); + + // unsupported level argument + assert_eq!( + cage.setsockopt_syscall(sockfd, SOL_UDP, SO_REUSEPORT, 0), + -(Errno::EOPNOTSUPP as i32) + ); + + // invalid optname argument + assert_eq!( + cage.setsockopt_syscall(sockfd, SOL_SOCKET, 20, 0), + -(Errno::EOPNOTSUPP as i32) + ); + assert_eq!( + cage.setsockopt_syscall(sockfd, SOL_TCP, 20, 0), + -(Errno::EOPNOTSUPP as i32) + ); + + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); + lindrustfinalize(); + } + #[test] pub fn ut_lind_net_socketoptions() { - //acquiring a lock on TESTMUTEX prevents other tests from running concurrently, + // acquiring a lock on TESTMUTEX prevents other tests from running concurrently, // and also performs clean env setup let _thelock = setup::lock_and_init(); @@ -3427,7 +3583,7 @@ pub mod net_tests { assert_eq!(cage.bind_syscall(sockfd, &socket), 0); assert_eq!(cage.listen_syscall(sockfd, 4), 0); - //set and get some options: + // set and get some options: let mut optstore = -12; assert_eq!( cage.getsockopt_syscall(sockfd, SOL_SOCKET, SO_REUSEPORT, &mut optstore), @@ -3445,7 +3601,7 @@ pub mod net_tests { ); assert_eq!(optstore, 0); - //linger... + // linger... assert_eq!( cage.getsockopt_syscall(sockfd, SOL_SOCKET, SO_LINGER, &mut optstore), 0 @@ -3458,7 +3614,7 @@ pub mod net_tests { ); assert_eq!(optstore, 1); - //check the options + // check the options assert_eq!( cage.getsockopt_syscall(sockfd, SOL_SOCKET, SO_REUSEPORT, &mut optstore), 0 @@ -3475,7 +3631,7 @@ pub mod net_tests { ); assert_eq!(optstore, 0); - //reuseport... + // reuseport... assert_eq!( cage.getsockopt_syscall(sockfd, SOL_SOCKET, SO_REUSEPORT, &mut optstore), 0 @@ -3491,7 +3647,7 @@ pub mod net_tests { ); assert_eq!(optstore, 1); - //check the options + // check the options assert_eq!( cage.getsockopt_syscall(sockfd, SOL_SOCKET, SO_REUSEPORT, &mut optstore), 0 @@ -3508,7 +3664,7 @@ pub mod net_tests { ); assert_eq!(optstore, 0); - //keep alive... + // keep alive... assert_eq!( cage.getsockopt_syscall(sockfd, SOL_SOCKET, SO_KEEPALIVE, &mut optstore), 0 @@ -3519,7 +3675,7 @@ pub mod net_tests { 0 ); - //check the options + // check the options assert_eq!( cage.getsockopt_syscall(sockfd, SOL_SOCKET, SO_REUSEPORT, &mut optstore), 0 @@ -3556,7 +3712,7 @@ pub mod net_tests { ); assert_eq!(optstore, 2000); - //check the options + // check the options assert_eq!( cage.getsockopt_syscall(sockfd, SOL_SOCKET, SO_REUSEPORT, &mut optstore), 0 @@ -3573,6 +3729,14 @@ pub mod net_tests { ); assert_eq!(optstore, 1); + // TCP_NODELAY for SOL_TCP + assert_eq!(cage.setsockopt_syscall(sockfd, SOL_TCP, TCP_NODELAY, 1), 0); + assert_eq!( + cage.getsockopt_syscall(sockfd, SOL_TCP, TCP_NODELAY, &mut optstore), + 0 + ); + assert_eq!(optstore, 1); + assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); lindrustfinalize(); } From 9385a31765ccad4fb46c1e0f3b4bad9d794e9ae4 Mon Sep 17 00:00:00 2001 From: Pranav Bhatt <42911524+pranav-bhatt@users.noreply.github.com> Date: Tue, 30 Jul 2024 00:12:54 +0530 Subject: [PATCH 3/5] Description, Comments and tests for `stat_syscall` & `fstat_syscall` (#301) * added comments for stat_syscall Signed-off-by: pranav * added the first few test cases * fstat_syscall documentation * completed test suite for stat_sycall * removed unnecessary import * FMT#1 * added fstat description + comments + tests * added incorporated comment feedback * reviewed documentation * corrected panic description in stat * fixed fstat panic bug * fixed fstat panic bug + modified test case to account for this * updated test case * addressed comments * addressed more comments * addressed more comments #2 * removed panic catch+unwind * removed extra import --------- Signed-off-by: pranav Co-authored-by: pranav --- src/safeposix/filesystem.rs | 8 ++ src/safeposix/syscalls/fs_calls.rs | 155 ++++++++++++++++++----- src/safeposix/syscalls/fs_constants.rs | 4 +- src/safeposix/syscalls/sys_calls.rs | 5 +- src/tests/fs_tests.rs | 167 ++++++++++++++++++++++--- src/tests/sys_tests.rs | 2 +- 6 files changed, 292 insertions(+), 49 deletions(-) diff --git a/src/safeposix/filesystem.rs b/src/safeposix/filesystem.rs index e3c37c7fc..8969e5fca 100644 --- a/src/safeposix/filesystem.rs +++ b/src/safeposix/filesystem.rs @@ -50,6 +50,8 @@ pub enum Inode { } #[derive(interface::SerdeSerialize, interface::SerdeDeserialize, Debug)] +/// Refer [here](https://man7.org/linux/man-pages/man7/inode.7.html) +/// for more information on the below fields. pub struct GenericInode { pub size: usize, pub uid: u32, @@ -66,6 +68,8 @@ pub struct GenericInode { } #[derive(interface::SerdeSerialize, interface::SerdeDeserialize, Debug)] +/// Refer [here](https://man7.org/linux/man-pages/man7/inode.7.html) +/// for more information on the below fields. pub struct DeviceInode { pub size: usize, pub uid: u32, @@ -83,6 +87,8 @@ pub struct DeviceInode { } #[derive(interface::SerdeSerialize, interface::SerdeDeserialize, Debug)] +/// Refer [here](https://man7.org/linux/man-pages/man7/inode.7.html) +/// for more information on the below fields. pub struct SocketInode { pub size: usize, pub uid: u32, @@ -97,6 +103,8 @@ pub struct SocketInode { } #[derive(interface::SerdeSerialize, interface::SerdeDeserialize, Debug)] +/// Refer [here](https://man7.org/linux/man-pages/man7/inode.7.html) +/// for more information on the below fields. pub struct DirectoryInode { pub size: usize, pub uid: u32, diff --git a/src/safeposix/syscalls/fs_calls.rs b/src/safeposix/syscalls/fs_calls.rs index bfd04ebc3..a1fb951c7 100644 --- a/src/safeposix/syscalls/fs_calls.rs +++ b/src/safeposix/syscalls/fs_calls.rs @@ -1148,12 +1148,47 @@ impl Cage { } //------------------------------------STAT SYSCALL------------------------------------ + /// ### Description + /// + /// `stat_syscall` retrieves file information for the file specified by + /// `path` and populates the provided `statbuf` with this information. + /// Although no file permissions are required to perform the call, execute + /// (search) permission is required on all of the directories in + /// pathname that lead to the file. + /// + /// ### Arguments + /// + /// It accepts two parameters: + /// * `path` - A string slice that specifies the file path for which status + /// information is to be retrieved. + /// * `statbuf` - A mutable reference to a `StatData` struct where the file + /// status will be stored. + /// + /// ### Returns + /// + /// For a successful call, the return value will be 0. On error, a negative + /// errno is returned to indicate the error. + /// + /// ### Errors + /// + /// * `ENOENT` - The file specified by `path` does not exist or the path is + /// invalid. + /// + /// ### Panics + /// + /// * This function does not have any known panics. + /// + /// For more detailed description of all the commands and return values, + /// refer to the stat man page [here](https://man7.org/linux/man-pages/man2/stat.2.html). pub fn stat_syscall(&self, path: &str, statbuf: &mut StatData) -> i32 { + //convert the path to an absolute path of type `PathBuf` let truepath = normpath(convpath(path), self); //Walk the file tree to get inode from path if let Some(inodenum) = metawalk(truepath.as_path()) { + // won't panic since check for inode number in table is already happening in + // above 'metawalk' function let inodeobj = FS_METADATA.inodetable.get(&inodenum).unwrap(); //populate those fields in statbuf which depend on things other than the inode @@ -1182,6 +1217,9 @@ impl Cage { } } + // helper function to populate information of generic inode objects (for example + // a file) into the statbuf. Refer [here](https://man7.org/linux/man-pages/man7/inode.7.html) + // for more information on the fields being populated below. fn _istat_helper(inodeobj: &GenericInode, statbuf: &mut StatData) { statbuf.st_mode = inodeobj.mode; statbuf.st_nlink = inodeobj.linkcount; @@ -1193,6 +1231,9 @@ impl Cage { statbuf.st_blocks = 0; } + // helper function to populate information of socket inode object into the + // statbuf. Refer [here](https://man7.org/linux/man-pages/man7/inode.7.html) + // for more information on the fields being populated below. fn _istat_helper_sock(inodeobj: &SocketInode, statbuf: &mut StatData) { statbuf.st_mode = inodeobj.mode; statbuf.st_nlink = inodeobj.linkcount; @@ -1204,6 +1245,9 @@ impl Cage { statbuf.st_blocks = 0; } + // helper function to populate information of directory inode object into the + // statbuf. Refer [here](https://man7.org/linux/man-pages/man7/inode.7.html) + // for more information on the fields being populated below. fn _istat_helper_dir(inodeobj: &DirectoryInode, statbuf: &mut StatData) { statbuf.st_mode = inodeobj.mode; statbuf.st_nlink = inodeobj.linkcount; @@ -1215,6 +1259,9 @@ impl Cage { statbuf.st_blocks = 0; } + // helper function to populate information of device inode object into the + // statbuf. Refer [here](https://man7.org/linux/man-pages/man7/inode.7.html) + // for more information on the fields being populated below. fn _istat_helper_chr_file(inodeobj: &DeviceInode, statbuf: &mut StatData) { statbuf.st_dev = 5; statbuf.st_mode = inodeobj.mode; @@ -1226,7 +1273,7 @@ impl Cage { statbuf.st_size = inodeobj.size; } - //Streams and pipes don't have associated inodes so we populate them from + // Streams and pipes don't have associated inodes so we populate them from // mostly dummy information fn _stat_alt_helper(&self, statbuf: &mut StatData, inodenum: usize) { statbuf.st_dev = FS_METADATA.dev_id; @@ -1242,18 +1289,67 @@ impl Cage { } //------------------------------------FSTAT SYSCALL------------------------------------ + /// ### Description + /// + /// `fstat_syscall` retrieves file status information for the file specified + /// by the file descriptor `fd` and populates the provided `statbuf` + /// with this information. + /// + /// ### Arguments + /// + /// It accepts two parameters: + /// * `fd` - The file descriptor for the file for which we need the status + /// information. + /// * `statbuf` - A mutable reference to a `StatData` struct where the file + /// status will be stored. + /// + /// ### Returns + /// + /// For a successful call, the return value will be 0. On error, a negative + /// errno is returned to indicate the error. + /// + /// ### Errors + /// + /// * `EBADF` - The file descriptor `fd` is invalid. + /// * `EOPNOTSUPP` - `fstat` is not supported on sockets. + /// + /// ### Panics + /// + /// * If the file descriptor passed is invalid (less than zero or greater + /// than MAX FD which is 1024) + /// * If the inode number retrieved from the file descriptor does not exist + /// in `FS_METADATA.inodetable`. + /// + /// For more detailed description of all the commands and return values, + /// refer to the stat man page [here](https://man7.org/linux/man-pages/man2/stat.2.html). pub fn fstat_syscall(&self, fd: i32, statbuf: &mut StatData) -> i32 { + // Attempt to get the file descriptor + // BUG: This can panic if there is an invalid file descriptor provided let checkedfd = self.get_filedescriptor(fd).unwrap(); + + // Acquire a write lock on the file descriptor to ensure exclusive access. let unlocked_fd = checkedfd.read(); + if let Some(filedesc_enum) = &*unlocked_fd { - //Delegate populating statbuf to the relevant helper depending on the file + // Delegate populating statbuf to the relevant helper depending on the file // type. First we check in the file descriptor to handle sockets, // streams, and pipes, and if it is a normal file descriptor we // handle regular files, dirs, and char files based on the // information in the inode. match filedesc_enum { + // Fail faster for sockets + Socket(_) => { + return syscall_error( + Errno::EOPNOTSUPP, + "fstat", + "we don't support fstat on sockets yet", + ); + } + + // if a normal file descriptor is found File(normalfile_filedesc_obj) => { + // fetch the inode object of the normal file let inode = FS_METADATA .inodetable .get(&normalfile_filedesc_obj.inode) @@ -1264,6 +1360,7 @@ impl Cage { statbuf.st_ino = normalfile_filedesc_obj.inode; statbuf.st_dev = FS_METADATA.dev_id; + // match inode to one of 4 inode types match &*inode { Inode::File(f) => { Self::_istat_helper(&f, statbuf); @@ -1279,21 +1376,17 @@ impl Cage { } } } - Socket(_) => { - return syscall_error( - Errno::EOPNOTSUPP, - "fstat", - "we don't support fstat on sockets yet", - ); - } + // Streams don't have inodes, so we'll populate statbuf with dummy info Stream(_) => { self._stat_alt_helper(statbuf, STREAMINODE); } + // Pipes don't have inodes, so we'll populate statbuf with dummy info Pipe(_) => { - self._stat_alt_helper(statbuf, 0xfeef0000); + self._stat_alt_helper(statbuf, PIPEINODE); } + // Epolls don't have inodes, so we'll populate statbuf with dummy info Epoll(_) => { - self._stat_alt_helper(statbuf, 0xfeef0000); + self._stat_alt_helper(statbuf, EPOLLINODE); } } 0 //fstat has succeeded! @@ -5212,40 +5305,40 @@ impl Cage { } /// ### Description - /// + /// /// `shmget_syscall` returns the shared memory segment identifier associated with a particular `key` /// If a key doesn't exist, shmget creates a new memory segment and attaches it to the key. - /// Traditionally if the value of the key equals `IPC_PRIVATE`, we also create a new memory segment which - /// is not associated with a key during this syscall, - /// but for our implementaion, we return an error and only create a new memory + /// Traditionally if the value of the key equals `IPC_PRIVATE`, we also create a new memory segment which + /// is not associated with a key during this syscall, + /// but for our implementaion, we return an error and only create a new memory /// segment when the IPC_CREAT flag is specified in the`shmflag` argument. - /// - /// ### Returns - /// + /// + /// ### Returns + /// /// An 32 bit integer which represens the identifier of the memory segment associated with the key - /// + /// /// ### Arguments - /// + /// /// `key` : An i32 value that references a memory segment /// `size` : Size of the memory segment to be created if key doesn't exist /// `shmflag` : mode flags which indicate whether to create a new key or not - /// The `shmflag` is composed of the following + /// The `shmflag` is composed of the following /// * IPC_CREAT - specify that the system call creates a new segment - /// * IPC_EXCL - this flag is used with IPC_CREAT to cause this function to fail when IPC_CREAT is also used + /// * IPC_EXCL - this flag is used with IPC_CREAT to cause this function to fail when IPC_CREAT is also used /// and the key passed has a memory segment associated with it. - /// - /// ### Errors - /// + /// + /// ### Errors + /// /// * ENOENT : the key equals the `IPC_PRIVATE` constant /// * EEXIST : key exists and yet either `IPC_CREAT` or `IPC_EXCL` are passed as flags /// * ENOENT : key did not exist and the `IPC_CREAT` flag was not passed /// * EINVAL : the size passed was less than the minimum size of segment or greater than the maximum possible size - /// + /// /// ### Panics - /// + /// /// There are no cases where the function directly panics - /// - pub fn shmget_syscall(&self, key: i32, size: usize, shmflg: i32) -> i32 { + /// + pub fn shmget_syscall(&self, key: i32, size: usize, shmflg: i32) -> i32 { //Check if the key passed equals the IPC_PRIVATE flag if key == IPC_PRIVATE { // Return error since this is not suppported currently @@ -5297,7 +5390,7 @@ impl Cage { // Insert new id in the hash table entry pointed by the key vacant.insert(shmid); // Mode of the new segment is the 9 least significant bits of the shmflag - let mode = (shmflg & 0x1FF) as u16; + let mode = (shmflg & 0x1FF) as u16; // Create a new segment with the key, size, cageid of the calling process let segment = new_shm_segment( key, @@ -5312,7 +5405,7 @@ impl Cage { } }; // Return the shmid - shmid + shmid } //------------------SHMAT SYSCALL------------------ diff --git a/src/safeposix/syscalls/fs_constants.rs b/src/safeposix/syscalls/fs_constants.rs index 1bbb85fc7..5ff5cb075 100644 --- a/src/safeposix/syscalls/fs_constants.rs +++ b/src/safeposix/syscalls/fs_constants.rs @@ -14,7 +14,9 @@ pub const STARTINGPIPE: i32 = 0; pub const MAXPIPE: i32 = 1024; pub const ROOTDIRECTORYINODE: usize = 1; -pub const STREAMINODE: usize = 2; +pub const STREAMINODE: usize = 2; // Dummy value +pub const PIPEINODE: usize = 0xfeef0000; // Dummy value +pub const EPOLLINODE: usize = 0xfeef0000; // Dummy value pub const PIPE_CAPACITY: usize = 65536; diff --git a/src/safeposix/syscalls/sys_calls.rs b/src/safeposix/syscalls/sys_calls.rs index 4aec87b99..10be51220 100644 --- a/src/safeposix/syscalls/sys_calls.rs +++ b/src/safeposix/syscalls/sys_calls.rs @@ -433,7 +433,7 @@ impl Cage { /// ### Panics /// /// This function doesn't directly panic - but the unmap memory mappings - /// function panics if it cannot create an shm entry or if the unwrapping + /// function panics if it cannot create an shm entry or if the unwrapping /// on the file descriptor fails due to an invalid fd /// /// For more information please refer to - [https://man7.org/linux/man-pages/man3/exec.3.html] @@ -479,7 +479,8 @@ impl Cage { // yet - And there is no threadId to store it at. // The child Cage object can then initialize and store the sigset appropriately // when it establishes its own main thread id. - // A sigset is a data structure that keeps track of which signals are affected by the process + // A sigset is a data structure that keeps track of which signals are affected + // by the process let newsigset = interface::RustHashMap::new(); if !interface::RUSTPOSIX_TESTSUITE.load(interface::RustAtomicOrdering::Relaxed) { // When rustposix runs independently (not as Lind paired with NaCL runtime) we diff --git a/src/tests/fs_tests.rs b/src/tests/fs_tests.rs index ed99bf4ae..0fba74b25 100644 --- a/src/tests/fs_tests.rs +++ b/src/tests/fs_tests.rs @@ -6,7 +6,7 @@ pub mod fs_tests { use crate::interface; use crate::safeposix::syscalls::fs_calls::*; use crate::safeposix::{cage::*, dispatcher::*, filesystem}; - use libc::c_void; + use libc::{c_void, O_DIRECTORY}; use std::fs::OpenOptions; use std::os::unix::fs::PermissionsExt; @@ -4038,31 +4038,46 @@ pub mod fs_tests { lindrustfinalize(); } - pub fn ut_lind_fs_shmget_syscall(){ + pub fn ut_lind_fs_shmget_syscall() { // acquire locks and start env cleanup let _thelock = setup::lock_and_init(); - let cage = interface::cagetable_getref(1); + let cage = interface::cagetable_getref(1); let key = 33123; // Get shmid of a memory segment / create a new one if it doesn't exist - let shmid = cage.shmget_syscall(33123, 1024, IPC_CREAT); - assert_eq!(shmid,4); + let shmid = cage.shmget_syscall(33123, 1024, IPC_CREAT); + assert_eq!(shmid, 4); // Check error upon asking for a valid key and passing the IPC_CREAT and IPC_EXCL flag - assert_eq!(cage.shmget_syscall(key, 1024, IPC_CREAT | IPC_EXCL),-(Errno::EEXIST as i32 )); + assert_eq!( + cage.shmget_syscall(key, 1024, IPC_CREAT | IPC_EXCL), + -(Errno::EEXIST as i32) + ); - // Check error when passing IPC_CREAT flag as the key - assert_eq!(cage.shmget_syscall(IPC_PRIVATE,1024,IPC_PRIVATE),-(Errno::ENOENT as i32)); + // Check error when passing IPC_CREAT flag as the key + assert_eq!( + cage.shmget_syscall(IPC_PRIVATE, 1024, IPC_PRIVATE), + -(Errno::ENOENT as i32) + ); - // Check if the function returns a correct shmid upon asking with a key that we know exists - assert_eq!(cage.shmget_syscall(key, 1024,0666),shmid); + // Check if the function returns a correct shmid upon asking with a key that we know exists + assert_eq!(cage.shmget_syscall(key, 1024, 0666), shmid); // Check if the function returns the correct error when we don't pass IPC_CREAT for a key that doesn't exist - assert_eq!(cage.shmget_syscall(123456, 1024, 0),-(Errno::ENOENT as i32)); + assert_eq!( + cage.shmget_syscall(123456, 1024, 0), + -(Errno::ENOENT as i32) + ); // Check if the size error is returned correctly - assert_eq!(cage.shmget_syscall(123456, (SHMMAX + 10 )as usize, IPC_CREAT),-(Errno::EINVAL as i32)); - assert_eq!(cage.shmget_syscall(123456, 0 as usize, IPC_CREAT),-(Errno::EINVAL as i32)); + assert_eq!( + cage.shmget_syscall(123456, (SHMMAX + 10) as usize, IPC_CREAT), + -(Errno::EINVAL as i32) + ); + assert_eq!( + cage.shmget_syscall(123456, 0 as usize, IPC_CREAT), + -(Errno::EINVAL as i32) + ); lindrustfinalize(); } @@ -4070,7 +4085,6 @@ pub mod fs_tests { #[test] pub fn ut_lind_fs_lseek_on_file() { // 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); @@ -4394,4 +4408,129 @@ pub mod fs_tests { assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); lindrustfinalize(); } + + #[test] + pub fn ut_lind_fs_stat_syscall_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 cage = interface::cagetable_getref(1); + let mut statdata = StatData::default(); + + // test out whether an error is output for a non existent file path + // (ENOENT[-2]) + assert_eq!( + cage.stat_syscall("non_existent_file_path", &mut statdata), + syscall_error(Errno::ENOENT, "stat", "test_failure") + ); + + // setting up directory inode object '/tmp' for testing stat_syscall with a + // directory + let dir_path = "/tmp"; // since setup already initializes tmp, assuming it is there + assert_eq!(cage.stat_syscall(dir_path, &mut statdata), 0); + + // setting up generic inode object "/tmp/generic" for testing stat_syscall with + // a generic file + let generic_path = "/tmp/generic"; + let creat_fd = cage.creat_syscall(generic_path, S_IRWXA); + assert!(creat_fd > 0); + assert_eq!(cage.stat_syscall(generic_path, &mut statdata), 0); + + // setting up character device inode object "/chardev" for testing stat_syscall + // with a character device + let dev = makedev(&DevNo { major: 1, minor: 3 }); + let chardev_path = "/chardev"; + assert_eq!( + cage.mknod_syscall(chardev_path, S_IRWXA | S_IFCHR as u32, dev), + 0 + ); + assert_eq!(cage.stat_syscall(chardev_path, &mut statdata), 0); + + // setting up socket inode object with path "/socket.sock" for testing + // stat_syscall with a socket + let socketfile_path = "/socket.sock"; + let socketfd = cage.socket_syscall(AF_UNIX, SOCK_STREAM, 0); + assert!(socketfd > 0); + let sockaddr = interface::new_sockaddr_unix(AF_UNIX as u16, socketfile_path.as_bytes()); + let socket = interface::GenSockaddr::Unix(sockaddr); + assert_eq!(cage.bind_syscall(socketfd, &socket), 0); + + // stat_syscall test here + assert_eq!(cage.stat_syscall(socketfile_path, &mut statdata), 0); + + // socket teardown + assert_eq!(cage.close_syscall(socketfd), 0); + cage.unlink_syscall(socketfile_path); + + lindrustfinalize(); + return; + } + + #[test] + pub fn ut_lind_fs_fstat_syscall_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 cage = interface::cagetable_getref(1); + + let mut statdata = StatData::default(); + + + // test out whether an error is output for a non existent fd (1000) + // (ENOENT[-2]) + let non_existent_fd = 1000; + assert_eq!(cage.fstat_syscall(non_existent_fd, &mut statdata), -9); + + // setting up directory inode object '/tmp' for testing fstat_syscall with a + // directory + let dir_path = "/tmp"; // since setup already initializes tmp, assuming it is there + let dir_fd = cage.open_syscall(dir_path, O_RDONLY | O_DIRECTORY, S_IRWXA); + assert!(dir_fd > 0); + assert_eq!(cage.fstat_syscall(dir_fd, &mut statdata), 0); + assert_eq!(cage.close_syscall(dir_fd), 0); + + // setting up generic inode object "/tmp/generic" for testing fstat_syscall with + // a generic file + let generic_path = "/tmp/generic"; + let creat_fd = cage.creat_syscall(generic_path, S_IRWXA); + assert!(creat_fd > 0); + assert_eq!(cage.fstat_syscall(creat_fd, &mut statdata), 0); + + // setting up character device inode object "/chardev" for testing fstat_syscall + // with a character device + let dev = makedev(&DevNo { major: 1, minor: 3 }); + let chardev_path = "/chardev"; + assert_eq!( + cage.mknod_syscall(chardev_path, S_IRWXA | S_IFCHR as u32, dev), + 0 + ); + let chardev_fd = cage.open_syscall(chardev_path, O_RDONLY, S_IRWXA); + assert!(chardev_fd > 0); + assert_eq!(cage.fstat_syscall(chardev_fd, &mut statdata), 0); + assert_eq!(cage.close_syscall(chardev_fd), 0); + + // setting up socket inode object with path "/socket.sock" for testing + // fstat_syscall with a socket + let socketfile_path = "/socket.sock"; + + let socketfd = cage.socket_syscall(AF_UNIX, SOCK_STREAM, 0); + assert!(socketfd > 0); + + let sockaddr = interface::new_sockaddr_unix(AF_UNIX as u16, socketfile_path.as_bytes()); + let socket = interface::GenSockaddr::Unix(sockaddr); + assert_eq!(cage.bind_syscall(socketfd, &socket), 0); + + // Errno::EOPNOTSUPP : -95 + assert_eq!(cage.fstat_syscall(socketfd, &mut statdata), -95); + + // Clean up + assert_eq!(cage.close_syscall(socketfd), 0); + + cage.unlink_syscall(socketfile_path); + + lindrustfinalize(); + return; + } } diff --git a/src/tests/sys_tests.rs b/src/tests/sys_tests.rs index 05e70d0c1..922a74f29 100644 --- a/src/tests/sys_tests.rs +++ b/src/tests/sys_tests.rs @@ -127,7 +127,7 @@ pub mod sys_tests { let _thelock = setup::lock_and_init(); let cage1 = interface::cagetable_getref(1); // Spawn a new child - cage1.fork_syscall(2); + cage1.fork_syscall(2); // Assert that the fork was correct let child_cage = interface::cagetable_getref(2); assert_eq!(child_cage.getuid_syscall(), -1); From 5c774e0c6732c198b53d7b41c2bc5a4d44bccb92 Mon Sep 17 00:00:00 2001 From: pranav Date: Tue, 30 Jul 2024 00:38:47 +0530 Subject: [PATCH 4/5] rebased with develop --- src/safeposix/syscalls/fs_calls.rs | 118 ++++++++++++++++++++++++----- src/tests/fs_tests.rs | 92 ++++++++++++++++++++-- 2 files changed, 186 insertions(+), 24 deletions(-) diff --git a/src/safeposix/syscalls/fs_calls.rs b/src/safeposix/syscalls/fs_calls.rs index a1fb951c7..7745284dc 100644 --- a/src/safeposix/syscalls/fs_calls.rs +++ b/src/safeposix/syscalls/fs_calls.rs @@ -1396,12 +1396,45 @@ impl Cage { } //------------------------------------STATFS SYSCALL------------------------------------ + /// ### Description + /// + /// `statfs_syscall` retrieves file system status information for the file + /// system containing the file specified by `path` and populates the + /// provided `databuf` with this information. + /// + /// ### Arguments + /// + /// It accepts two parameters: + /// * `path` - A string slice that specifies the file path for which file + /// system status information is to be retrieved. + /// * `databuf` - A mutable reference to a `FSData` struct where the file + /// system status will be stored. + /// + /// ### Returns + /// + /// For a successful call, the return value will be 0. On error, a negative + /// errno is returned to indicate the error. + /// + /// ### Errors + /// + /// * `ENOENT` - The file specified by `path` does not exist or the path is + /// invalid. + /// + /// ### Panics + /// + /// * There are no panics that can happen in this function. + /// + /// For more detailed description of all the commands and return values, + /// refer to the statfs man page [here](https://man7.org/linux/man-pages/man2/statfs.2.html). pub fn statfs_syscall(&self, path: &str, databuf: &mut FSData) -> i32 { + //convert the path to an absolute path of type `PathBuf` let truepath = normpath(convpath(path), self); //Walk the file tree to get inode from path if let Some(inodenum) = metawalk(truepath.as_path()) { + // won't panic since check for inode number in table is already happening in + // above 'metawalk' function let _inodeobj = FS_METADATA.inodetable.get(&inodenum).unwrap(); //populate the dev id field -- can be done outside of the helper @@ -1415,35 +1448,80 @@ impl Cage { } //------------------------------------FSTATFS SYSCALL------------------------------------ + /// ### Description + /// + /// `fstatfs_syscall` retrieves file system status information for the file + /// system containing the file specified by the file descriptor `fd` and + /// populates the provided `databuf` with this information. + /// + /// ### Arguments + /// + /// It accepts two parameters: + /// * `fd` - The file descriptor that refers to the open file. + /// * `databuf` - A mutable reference to a `FSData` struct where the file + /// system status will be stored. + /// + /// ### Returns + /// + /// For a successful call, the return value will be 0. On error, a negative + /// errno is returned to indicate the error. + /// + /// ### Errors + /// + /// * `EBADF` - The file descriptor `fd` is either invalid or it refers to a + /// socket, stream, pipe, or epoll file descriptor, which are not + /// supported by this function. + /// + /// ### Panics + /// + /// * If the file descriptor provided isn't in the appropriate range, this + /// function can panic. + /// + /// For more detailed description of all the commands and return values, + /// refer to the statfs man page [here](https://man7.org/linux/man-pages/man2/statfs.2.html). pub fn fstatfs_syscall(&self, fd: i32, databuf: &mut FSData) -> i32 { + // BUG: If the provided file descriptor is out of bounds, get_filedescriptor + // returns Err(), unwrapping on which produces a 'panic!' + // otherwise, file descriptor table entry is stored in 'checkedfd' let checkedfd = self.get_filedescriptor(fd).unwrap(); let unlocked_fd = checkedfd.read(); + if let Some(filedesc_enum) = &*unlocked_fd { //populate the dev id field -- can be done outside of the helper databuf.f_fsid = FS_METADATA.dev_id; match filedesc_enum { + // if the fd points to a normal file descriptor File(normalfile_filedesc_obj) => { + // won't panic since we have already checked that the entries exist + // in the FS_METADATA inode table let _inodeobj = FS_METADATA .inodetable .get(&normalfile_filedesc_obj.inode) .unwrap(); + // populate the databuf using a helper function return Self::_istatfs_helper(self, databuf); } + + // if the fd points to a socket, pipe, stream, or epoll file descriptor Socket(_) | Pipe(_) | Stream(_) | Epoll(_) => { return syscall_error( Errno::EBADF, "fstatfs", - "can't fstatfs on socket, stream, pipe, or epollfd", + "can't fstatfs on sockets, streams, pipes, or epoll fds", ); } } + } else { + return syscall_error(Errno::EBADF, "statfs", "invalid file descriptor"); } - return syscall_error(Errno::EBADF, "statfs", "invalid file descriptor"); } + // These values have (probably) been picked up from the previously used + // environment, and have been working fine till now for our purposes + // TODO: Figure out how to populate the databuf values properly pub fn _istatfs_helper(&self, databuf: &mut FSData) -> i32 { databuf.f_type = 0xBEEFC0DE; //unassigned databuf.f_bsize = 4096; @@ -5306,16 +5384,19 @@ impl Cage { /// ### Description /// - /// `shmget_syscall` returns the shared memory segment identifier associated with a particular `key` - /// If a key doesn't exist, shmget creates a new memory segment and attaches it to the key. - /// Traditionally if the value of the key equals `IPC_PRIVATE`, we also create a new memory segment which - /// is not associated with a key during this syscall, - /// but for our implementaion, we return an error and only create a new memory - /// segment when the IPC_CREAT flag is specified in the`shmflag` argument. + /// `shmget_syscall` returns the shared memory segment identifier associated + /// with a particular `key` If a key doesn't exist, shmget creates a new + /// memory segment and attaches it to the key. Traditionally if the + /// value of the key equals `IPC_PRIVATE`, we also create a new memory + /// segment which is not associated with a key during this syscall, + /// but for our implementaion, we return an error and only create a new + /// memory segment when the IPC_CREAT flag is specified in the`shmflag` + /// argument. /// /// ### Returns /// - /// An 32 bit integer which represens the identifier of the memory segment associated with the key + /// An 32 bit integer which represens the identifier of the memory segment + /// associated with the key /// /// ### Arguments /// @@ -5324,20 +5405,22 @@ impl Cage { /// `shmflag` : mode flags which indicate whether to create a new key or not /// The `shmflag` is composed of the following /// * IPC_CREAT - specify that the system call creates a new segment - /// * IPC_EXCL - this flag is used with IPC_CREAT to cause this function to fail when IPC_CREAT is also used - /// and the key passed has a memory segment associated with it. + /// * IPC_EXCL - this flag is used with IPC_CREAT to cause this function to + /// fail when IPC_CREAT is also used and the key passed has a memory + /// segment associated with it. /// /// ### Errors /// /// * ENOENT : the key equals the `IPC_PRIVATE` constant - /// * EEXIST : key exists and yet either `IPC_CREAT` or `IPC_EXCL` are passed as flags + /// * EEXIST : key exists and yet either `IPC_CREAT` or `IPC_EXCL` are + /// passed as flags /// * ENOENT : key did not exist and the `IPC_CREAT` flag was not passed - /// * EINVAL : the size passed was less than the minimum size of segment or greater than the maximum possible size + /// * EINVAL : the size passed was less than the minimum size of segment or + /// greater than the maximum possible size /// /// ### Panics /// /// There are no cases where the function directly panics - /// pub fn shmget_syscall(&self, key: i32, size: usize, shmflg: i32) -> i32 { //Check if the key passed equals the IPC_PRIVATE flag if key == IPC_PRIVATE { @@ -5349,7 +5432,8 @@ impl Cage { // data of the shm table let metadata = &SHM_METADATA; - // Check if there exists a memory segment associated with the key passed as argument + // Check if there exists a memory segment associated with the key passed as + // argument match metadata.shmkeyidtable.entry(key) { // If there exists a memory segment at that key interface::RustHashEntry::Occupied(occupied) => { @@ -5375,8 +5459,8 @@ impl Cage { ); } - // If memory segment doesn't exist and IPC_CREAT was specified - we create a new memory segment - // Check if the size passed is a valid value + // If memory segment doesn't exist and IPC_CREAT was specified - we create a new + // memory segment Check if the size passed is a valid value if (size as u32) < SHMMIN || (size as u32) > SHMMAX { return syscall_error( Errno::EINVAL, diff --git a/src/tests/fs_tests.rs b/src/tests/fs_tests.rs index 0fba74b25..7e87ec0fc 100644 --- a/src/tests/fs_tests.rs +++ b/src/tests/fs_tests.rs @@ -4034,7 +4034,6 @@ pub mod fs_tests { assert_eq!(cage.close_syscall(fd), 0); assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); - lindrustfinalize(); } @@ -4048,7 +4047,8 @@ pub mod fs_tests { let shmid = cage.shmget_syscall(33123, 1024, IPC_CREAT); assert_eq!(shmid, 4); - // Check error upon asking for a valid key and passing the IPC_CREAT and IPC_EXCL flag + // Check error upon asking for a valid key and passing the IPC_CREAT and + // IPC_EXCL flag assert_eq!( cage.shmget_syscall(key, 1024, IPC_CREAT | IPC_EXCL), -(Errno::EEXIST as i32) @@ -4060,10 +4060,12 @@ pub mod fs_tests { -(Errno::ENOENT as i32) ); - // Check if the function returns a correct shmid upon asking with a key that we know exists + // Check if the function returns a correct shmid upon asking with a key that we + // know exists assert_eq!(cage.shmget_syscall(key, 1024, 0666), shmid); - // Check if the function returns the correct error when we don't pass IPC_CREAT for a key that doesn't exist + // Check if the function returns the correct error when we don't pass IPC_CREAT + // for a key that doesn't exist assert_eq!( cage.shmget_syscall(123456, 1024, 0), -(Errno::ENOENT as i32) @@ -4408,7 +4410,7 @@ pub mod fs_tests { assert_eq!(cage.exit_syscall(EXIT_SUCCESS), EXIT_SUCCESS); lindrustfinalize(); } - + #[test] pub fn ut_lind_fs_stat_syscall_tests() { // acquiring a lock on TESTMUTEX prevents other tests from running concurrently, @@ -4477,7 +4479,6 @@ pub mod fs_tests { let mut statdata = StatData::default(); - // test out whether an error is output for a non existent fd (1000) // (ENOENT[-2]) let non_existent_fd = 1000; @@ -4522,7 +4523,7 @@ pub mod fs_tests { let socket = interface::GenSockaddr::Unix(sockaddr); assert_eq!(cage.bind_syscall(socketfd, &socket), 0); - // Errno::EOPNOTSUPP : -95 + // Errno::EOPNOTSUPP : -95 assert_eq!(cage.fstat_syscall(socketfd, &mut statdata), -95); // Clean up @@ -4533,4 +4534,81 @@ pub mod fs_tests { lindrustfinalize(); return; } + + #[test] + pub fn ut_lind_fs_statfs_syscall_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 cage = interface::cagetable_getref(1); + let mut fsdata = FSData::default(); + + // test out whether an error is output for a non existent file path + // (ENOENT[-2]) + assert_eq!( + cage.statfs_syscall("non_existent_file_path", &mut fsdata), + syscall_error(Errno::ENOENT, "stat", "test_failure") + ); + + // setting up inode object "/tmp/generic" for testing statfs_syscall + let generic_path = "/tmp/generic"; + let creat_fd = cage.creat_syscall(generic_path, S_IRWXA); + assert!(creat_fd > 0); + assert_eq!(cage.statfs_syscall(generic_path, &mut fsdata), 0); + + lindrustfinalize(); + return; + } + + #[test] + pub fn ut_lind_fs_fstatfs_syscall_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 cage = interface::cagetable_getref(1); + + let mut fsdata = FSData::default(); + + // test out whether an error is output for a non existent fd (1000) + // (ENOENT[-2]) + let non_existent_fd = 1000; + assert_eq!( + cage.fstatfs_syscall(non_existent_fd, &mut fsdata), + syscall_error(Errno::EBADF, "stat", "test_failure") + ); + + // setting up generic inode object "/tmp/generic" for testing fstat_syscall with + // a generic file + let generic_path = "/tmp/generic"; + let creat_fd = cage.creat_syscall(generic_path, S_IRWXA); + assert!(creat_fd > 0); + assert_eq!(cage.fstatfs_syscall(creat_fd, &mut fsdata), 0); + + // setting up socket inode object with path "/socket.sock" for testing + // fstat_syscall with a socket + let socketfile_path = "/socket.sock"; + + let socketfd = cage.socket_syscall(AF_UNIX, SOCK_STREAM, 0); + assert!(socketfd > 0); + + let sockaddr = interface::new_sockaddr_unix(AF_UNIX as u16, socketfile_path.as_bytes()); + let socket = interface::GenSockaddr::Unix(sockaddr); + assert_eq!(cage.bind_syscall(socketfd, &socket), 0); + + // Errno::EBADF : -9 + assert_eq!( + cage.fstatfs_syscall(socketfd, &mut fsdata), + syscall_error(Errno::EBADF, "stat", "test_failure") + ); + + // Clean up + assert_eq!(cage.close_syscall(socketfd), 0); + + cage.unlink_syscall(socketfile_path); + + lindrustfinalize(); + return; + } } From bd22ba9cd26f7d25f9cbe34894287a31e1966d7b Mon Sep 17 00:00:00 2001 From: Rupesh Koushik <108213465+rupeshkoushik07@users.noreply.github.com> Date: Wed, 31 Jul 2024 08:58:12 +0530 Subject: [PATCH 5/5] fix warnings (#318) --- src/lib.rs | 1 - src/main.rs | 1 - src/tools/fs_utils.rs | 1 - 3 files changed, 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index fb24650f9..336aed325 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -1,4 +1,3 @@ -#![feature(lazy_cell)] #![feature(rustc_private)] //for private crate imports for tests #![feature(vec_into_raw_parts)] #![feature(thread_local)] diff --git a/src/main.rs b/src/main.rs index 508ab47d7..b2ab4f1aa 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,4 +1,3 @@ -#![feature(lazy_cell)] #![feature(rustc_private)] //for private crate imports for tests #![feature(vec_into_raw_parts)] #![feature(duration_constants)] diff --git a/src/tools/fs_utils.rs b/src/tools/fs_utils.rs index de96488ab..b5bc04925 100644 --- a/src/tools/fs_utils.rs +++ b/src/tools/fs_utils.rs @@ -1,4 +1,3 @@ -#![feature(lazy_cell)] #![feature(rustc_private)] //for private crate imports for tests #![feature(vec_into_raw_parts)] #![feature(duration_constants)]