Skip to content
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
merged 15 commits into from
Oct 22, 2024

Conversation

ChinmayShringi
Copy link
Contributor

Fixes # (issue)

This PR addresses a critical issue in the mmap system call implementation where both MAP_PRIVATE and MAP_SHARED flags were not properly checked for mutual exclusivity. The changes ensure that the mmap system call returns an EINVAL 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 that errno is set to EINVAL when the invalid flag combination is passed.

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

  • Test A: cargo test tests::fs_tests::fs_tests::ut_lind_fs_mmap_invalid_flags_both
    • This test verifies that the mmap system call correctly handles the invalid flag combination and returns the appropriate EINVAL error.

Checklist:

  • My code follows the style guidelines of this project.
  • I have commented my code, particularly in hard-to-understand areas.
  • My changes generate no new warnings.
  • I have added tests that prove my fix is effective.
  • Any dependent changes have been added to a pull request and/or merged in other modules (native-client, lind-glibc, lind-project).

Yaxuan-w and others added 10 commits September 16, 2024 16:15
…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
@ChinmayShringi ChinmayShringi changed the title fix: test case ut-lind-fs-mmap-invalid-flags-both Fix mmap test case to handle invalid flags scenario correctly Sep 23, 2024
Copy link
Member

@Yaxuan-w Yaxuan-w left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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");
}
Copy link
Member

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>
@ChinmayShringi ChinmayShringi changed the base branch from porting-fdtables to main October 8, 2024 19:21
ChinmayShringi and others added 3 commits October 8, 2024 15:24
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 Yaxuan-w merged commit 1e4fe90 into main Oct 22, 2024
1 check failed
@Yaxuan-w Yaxuan-w deleted the fix-ut-lind-fs-mmap-invalid-flags-both branch October 25, 2024 21:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants