Skip to content

Conversation

@DeathWish5
Copy link
Contributor

@DeathWish5 DeathWish5 commented Jul 14, 2020

Add right checks in socket and pass the rest core-tests (ReadIntoBadBuffer、WriteFromBadBuffer).
But there is some doubts about my change in zircon-syscall::vmar::sys_vmar_map.

// if !options.contains(VmOptions::PERM_READ)
//     && (!options.contains(VmOptions::PERM_WRITE)
//         || options.contains(VmOptions::PERM_EXECUTE))
// {
//     return Err(ZxError::INVALID_ARGS);
// }

I comment these codes which make sense because in core-tests of socket, an address is mapped as not readable, not writable and not executable, which should be ok but leads to errors due to the code above. However, a page with no rights is useless and only occurs in tests.

mapping_flags.set(
    MMUFlags::READ,
//  vmar_rights.contains(Rights::READ) && vmo_rights.contains(Rights::READ),
    vmar_rights.contains(Rights::READ) && vmo_rights.contains(Rights::READ) && options.contains(VmOptions::PERM_READ),
);
mapping_flags.set(
    MMUFlags::WRITE,
//  vmar_rights.contains(Rights::WRITE) && vmo_rights.contains(Rights::WRITE),
    vmar_rights.contains(Rights::WRITE) && vmo_rights.contains(Rights::WRITE) && options.contains(VmOptions::PERM_WRITE),
);
mapping_flags.set(
    MMUFlags::EXECUTE,
//  vmar_rights.contains(Rights::EXECUTE) && vmo_rights.contains(Rights::EXECUTE),
    vmar_rights.contains(Rights::EXECUTE) && vmo_rights.contains(Rights::EXECUTE) && options.contains(VmOptions::PERM_EXECUTE),
);

The implementation in zircon is same as zCore which doesn't care options and perm_xxxx. I am clear about the meanings of options::perm_xxxx, but without them, the rest core-tests will fail. (QAQ
😓😓😓😓

@coveralls
Copy link

coveralls commented Jul 14, 2020

Pull Request Test Coverage Report for Build 168743355

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 22 (0.0%) changed or added relevant lines in 4 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.004%) to 29.156%

Changes Missing Coverage Covered Lines Changed/Added Lines %
zircon-syscall/src/vmar.rs 0 3 0.0%
zircon-object/src/vm/vmar.rs 0 5 0.0%
zircon-object/src/ipc/socket.rs 0 7 0.0%
zircon-syscall/src/socket.rs 0 7 0.0%
Files with Coverage Reduction New Missed Lines %
zircon-object/src/object/mod.rs 1 81.55%
Totals Coverage Status
Change from base Build 166307932: -0.004%
Covered Lines: 3490
Relevant Lines: 11970

💛 - Coveralls

@chyyuu
Copy link
Member

chyyuu commented Aug 14, 2021

need more work!

@chyyuu chyyuu closed this Aug 14, 2021
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.

3 participants