-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix mmap test case to handle invalid flags scenario correctly #22
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…t case (#16) * fix: ut_lind_fs_chdir_removeddir * fix: add syscalls * fix: revert changes * fix: testing subdir1 * fix: testing without i32 * fix: revert to mkdir * fix: test with if * fix: revert * debug: added print * debug: added print * fix: print * fix: revert println * feat: return invalid * fix: revert removed return * feat: update rm subdir1 subdir2 * refactor: update comments
Yaxuan-w
approved these changes
Sep 23, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Yaxuan-w
reviewed
Sep 27, 2024
src/safeposix/syscalls/fs_calls.rs
Outdated
Comment on lines
776
to
779
// returns an EINVAL error if both flags are set simultaneously | ||
if (flags & libc::MAP_PRIVATE != 0) && (flags & libc::MAP_SHARED != 0) { | ||
return syscall_error(Errno::EINVAL, "mmap", "Invalid flags: both MAP_PRIVATE and MAP_SHARED set"); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this case is handled by libc::mmap
? Could you check that?
commit e44f2c0 Author: Alice Wen <40227173+Yaxuan-w@users.noreply.github.com> Date: Tue Oct 8 14:58:46 2024 -0400 Update README.md commit e244b91 Author: Chinmay Shringi <31031919+ChinmayShringi@users.noreply.github.com> Date: Tue Oct 8 14:55:53 2024 -0400 Feat update readme (#49) * feat: update readme * feat: added images commit ebff6b7 Author: Alice Wen <40227173+Yaxuan-w@users.noreply.github.com> Date: Tue Oct 1 13:21:00 2024 -0400 Porting newest version fdtables and Synchronize the progress of porting test suite (#14) * change to latest fdtables * using newest fdtables * mmap debug * mmap debug * mmap debug * porting test suite * porting test suite * Add more detailed comments * Fix directory cleanup issue in ut_lind_fs_mmap_invalid_flags_both test case (#16) * fix: ut_lind_fs_chdir_removeddir * fix: add syscalls * fix: revert changes * fix: testing subdir1 * fix: testing without i32 * fix: revert to mkdir * fix: test with if * fix: revert * debug: added print * debug: added print * fix: print * fix: revert println * feat: return invalid * fix: revert removed return * feat: update rm subdir1 subdir2 * refactor: update comments * fix: test case ut-lind-fs-fchdir-invalid-args * Fix out-of-range file descriptor error in `getdents` syscall (#17) * Fix: out-of-range file descriptor handling in getdents syscall * fix: update comment * Update src/fdtables/dashmapvecglobal.rs Co-authored-by: Justin Cappos <justincappos@gmail.com> --------- Co-authored-by: lind <lind@nyu.edu> Co-authored-by: Justin Cappos <justincappos@gmail.com> * Fix `getcwd_syscall` to Handle Invalid Arguments and Correctly Set `errno` (#24) * fix: test case ut_lind_fs_getcwd_invalid_args * fix: PR comments --------- Co-authored-by: lind <lind@nyu.edu> * Fix mmap test case to handle invalid flags scenario correctly (#18) * Fix: mmap test to validate EINVAL for invalid flags * fix: PR comments --------- Co-authored-by: lind <lind@nyu.edu> * Fix Test `ut_lind_fs_pread_from_directory` from Directory to Return Correct Error (#27) * fix: test case ut_lind_fs_pread_from_directory * fix: add rmdir syscall --------- Co-authored-by: lind <lind@nyu.edu> * fix: test case ut-lind-fs-write-to-directory (#19) Co-authored-by: lind <lind@nyu.edu> * fix: ut-lind-fs-simple (#21) Co-authored-by: lind <lind@nyu.edu> * Fix `ut_lind_fs_rmdir_nowriteperm_parent_dir` rmdir Nowrite Permission Handling and Recursive Directory Cleanup (#28) * fix: test case ut_lind_fs_rmdir_nowriteperm_parent_dir * feat: added cleanup --------- Co-authored-by: lind <lind@nyu.edu> * Fix ut lind fs mkdir nonexisting directory (#25) * fix: test case ut_lind_fs_mkdir_nonexisting_directory * feat: added rmdir_recursive_syscall * fix: rm unecessary function * feat: updated comment * feat: updated values --------- Co-authored-by: lind <lind@nyu.edu> * Fix EISDIR Error in `ut_lind_fs_getdents_bufsize_too_small` Test by Adjusting Directory Open Flags (#29) * fix: test case ut_lind_fs_getdents_bufsize_too_small * fix: rmdir for clean env --------- Co-authored-by: lind <lind@nyu.edu> * Fix EEXIST Error in `ut_lind_fs_close_directory` Test by Removing Existing Directories Before Creation (#31) * fix: test case ut_lind_fs_close_directory * feat: use rmdir --------- Co-authored-by: lind <lind@nyu.edu> * Fix EEXIST Error in `ut_lind_fs_dir_multiple` Test by Removing Existing Directories Before Creation (#32) * fix: test case ut_lind_fs_dir_multiple * feat: updated to rmdir --------- Co-authored-by: lind <lind@nyu.edu> * Fix EEXIST Error in `ut_lind_fs_dir_mode` Test by Removing Existing Directories Before Creation (#30) * fix: test case ut_lind_fs_dir_mode * fix: cleanup the enviornment --------- Co-authored-by: lind <lind@nyu.edu> * Fix EEXIST Error in `ut_lind_fs_unlink_directory` (#34) * fix: test case ut_lind_fs_rmdir_nonexist_dir (#36) * fix: test case ut_lind_fs_rmdir_nonempty_dir (#37) * fix: test case ut_lind_fs_link_directory (#38) * fix: test case ut_lind_fs_read_from_directory (#39) * fix: test case ut_lind_fs_rename (#41) * Fix in `ut_lind_fs_tmp_file_test` ensure /tmp Directory Is Properly Set Up and Cleaned (#35) * fix: test case ut_lind_fs_read_from_chardev_file (#44) Co-authored-by: lind <lind@nyu.edu> --------- Co-authored-by: Chinmay Shringi <31031919+ChinmayShringi@users.noreply.github.com> Co-authored-by: lind <lind@nyu.edu> Co-authored-by: Nicholas Renner <nr2185@nyu.edu> Co-authored-by: Justin Cappos <justincappos@gmail.com>
commit e44f2c0 Author: Alice Wen <40227173+Yaxuan-w@users.noreply.github.com> Date: Tue Oct 8 14:58:46 2024 -0400 Update README.md commit e244b91 Author: Chinmay Shringi <31031919+ChinmayShringi@users.noreply.github.com> Date: Tue Oct 8 14:55:53 2024 -0400 Feat update readme (#49) * feat: update readme * feat: added images commit ebff6b7 Author: Alice Wen <40227173+Yaxuan-w@users.noreply.github.com> Date: Tue Oct 1 13:21:00 2024 -0400 Porting newest version fdtables and Synchronize the progress of porting test suite (#14) * change to latest fdtables * using newest fdtables * mmap debug * mmap debug * mmap debug * porting test suite * porting test suite * Add more detailed comments * Fix directory cleanup issue in ut_lind_fs_mmap_invalid_flags_both test case (#16) * fix: ut_lind_fs_chdir_removeddir * fix: add syscalls * fix: revert changes * fix: testing subdir1 * fix: testing without i32 * fix: revert to mkdir * fix: test with if * fix: revert * debug: added print * debug: added print * fix: print * fix: revert println * feat: return invalid * fix: revert removed return * feat: update rm subdir1 subdir2 * refactor: update comments * fix: test case ut-lind-fs-fchdir-invalid-args * Fix out-of-range file descriptor error in `getdents` syscall (#17) * Fix: out-of-range file descriptor handling in getdents syscall * fix: update comment * Update src/fdtables/dashmapvecglobal.rs Co-authored-by: Justin Cappos <justincappos@gmail.com> --------- Co-authored-by: lind <lind@nyu.edu> Co-authored-by: Justin Cappos <justincappos@gmail.com> * Fix `getcwd_syscall` to Handle Invalid Arguments and Correctly Set `errno` (#24) * fix: test case ut_lind_fs_getcwd_invalid_args * fix: PR comments --------- Co-authored-by: lind <lind@nyu.edu> * Fix mmap test case to handle invalid flags scenario correctly (#18) * Fix: mmap test to validate EINVAL for invalid flags * fix: PR comments --------- Co-authored-by: lind <lind@nyu.edu> * Fix Test `ut_lind_fs_pread_from_directory` from Directory to Return Correct Error (#27) * fix: test case ut_lind_fs_pread_from_directory * fix: add rmdir syscall --------- Co-authored-by: lind <lind@nyu.edu> * fix: test case ut-lind-fs-write-to-directory (#19) Co-authored-by: lind <lind@nyu.edu> * fix: ut-lind-fs-simple (#21) Co-authored-by: lind <lind@nyu.edu> * Fix `ut_lind_fs_rmdir_nowriteperm_parent_dir` rmdir Nowrite Permission Handling and Recursive Directory Cleanup (#28) * fix: test case ut_lind_fs_rmdir_nowriteperm_parent_dir * feat: added cleanup --------- Co-authored-by: lind <lind@nyu.edu> * Fix ut lind fs mkdir nonexisting directory (#25) * fix: test case ut_lind_fs_mkdir_nonexisting_directory * feat: added rmdir_recursive_syscall * fix: rm unecessary function * feat: updated comment * feat: updated values --------- Co-authored-by: lind <lind@nyu.edu> * Fix EISDIR Error in `ut_lind_fs_getdents_bufsize_too_small` Test by Adjusting Directory Open Flags (#29) * fix: test case ut_lind_fs_getdents_bufsize_too_small * fix: rmdir for clean env --------- Co-authored-by: lind <lind@nyu.edu> * Fix EEXIST Error in `ut_lind_fs_close_directory` Test by Removing Existing Directories Before Creation (#31) * fix: test case ut_lind_fs_close_directory * feat: use rmdir --------- Co-authored-by: lind <lind@nyu.edu> * Fix EEXIST Error in `ut_lind_fs_dir_multiple` Test by Removing Existing Directories Before Creation (#32) * fix: test case ut_lind_fs_dir_multiple * feat: updated to rmdir --------- Co-authored-by: lind <lind@nyu.edu> * Fix EEXIST Error in `ut_lind_fs_dir_mode` Test by Removing Existing Directories Before Creation (#30) * fix: test case ut_lind_fs_dir_mode * fix: cleanup the enviornment --------- Co-authored-by: lind <lind@nyu.edu> * Fix EEXIST Error in `ut_lind_fs_unlink_directory` (#34) * fix: test case ut_lind_fs_rmdir_nonexist_dir (#36) * fix: test case ut_lind_fs_rmdir_nonempty_dir (#37) * fix: test case ut_lind_fs_link_directory (#38) * fix: test case ut_lind_fs_read_from_directory (#39) * fix: test case ut_lind_fs_rename (#41) * Fix in `ut_lind_fs_tmp_file_test` ensure /tmp Directory Is Properly Set Up and Cleaned (#35) * fix: test case ut_lind_fs_read_from_chardev_file (#44) Co-authored-by: lind <lind@nyu.edu> --------- Co-authored-by: Chinmay Shringi <31031919+ChinmayShringi@users.noreply.github.com> Co-authored-by: lind <lind@nyu.edu> Co-authored-by: Nicholas Renner <nr2185@nyu.edu> Co-authored-by: Justin Cappos <justincappos@gmail.com> Merge branch 'main' into fix-ut-lind-fs-mmap-invalid-flags-both
Yaxuan-w
approved these changes
Oct 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes # (issue)
This PR addresses a critical issue in the
mmap
system call implementation where bothMAP_PRIVATE
andMAP_SHARED
flags were not properly checked for mutual exclusivity. The changes ensure that themmap
system call returns anEINVAL
error if both flags are set simultaneously, which aligns with the POSIX standard.Additionally, changes were made in the
ut_lind_fs_mmap_invalid_flags_none
test case to validate this behavior. The test now checks for the generic error code-1
and verifies thaterrno
is set toEINVAL
when the invalid flag combination is passed.Type of change
How Has This Been Tested?
cargo test tests::fs_tests::fs_tests::ut_lind_fs_mmap_invalid_flags_both
mmap
system call correctly handles the invalid flag combination and returns the appropriateEINVAL
error.Checklist: