Skip to content

Don't use PROT_EXEC with mmap on macos #1110

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 15, 2024
Merged

Don't use PROT_EXEC with mmap on macos #1110

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/util/memory.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ pub fn set(start: Address, val: u8, len: usize) {
/// may corrupt others' data.
#[allow(clippy::let_and_return)] // Zeroing is not neceesary for some OS/s
pub unsafe fn dzmmap(start: Address, size: usize, strategy: MmapStrategy) -> Result<()> {
let prot = PROT_READ | PROT_WRITE | PROT_EXEC;
let prot = MMAP_PROT;
let flags = libc::MAP_ANON | libc::MAP_PRIVATE | libc::MAP_FIXED;
let ret = mmap_fixed(start, size, prot, flags, strategy);
// We do not need to explicitly zero for Linux (memory is guaranteed to be zeroed)
Expand All @@ -58,6 +58,12 @@ const MMAP_FLAGS: libc::c_int = libc::MAP_ANON | libc::MAP_PRIVATE | libc::MAP_F
// MAP_FIXED is used instead of MAP_FIXED_NOREPLACE (which is not available on macOS). We are at the risk of overwriting pre-existing mappings.
const MMAP_FLAGS: libc::c_int = libc::MAP_ANON | libc::MAP_PRIVATE | libc::MAP_FIXED;

#[cfg(target_os = "linux")]
const MMAP_PROT: libc::c_int = PROT_READ | PROT_WRITE | PROT_EXEC;
#[cfg(target_os = "macos")]
// PROT_EXEC cannot be used with PROT_READ on Apple Silicon
const MMAP_PROT: libc::c_int = PROT_READ | PROT_WRITE;
Copy link
Member

@caizixian caizixian Apr 12, 2024

Choose a reason for hiding this comment

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

Thanks for the PR. I think for now getting something to work is better than nothing. But we do need to consider how MMTk handles code allocation for JIT.

MMTk might need to do this on macOS https://developer.apple.com/documentation/apple-silicon/porting-just-in-time-compilers-to-apple-silicon#Enable-the-JIT-entitlements-for-the-Hardened-Runtime

Copy link
Collaborator

@k-sareen k-sareen Apr 15, 2024

Choose a reason for hiding this comment

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

Thank you for the PR! I wonder if we should do things differently for Intel Macs vs Apple Silicon? Though maybe it makes more sense to have the same mechanism regardless of the underlying architecture. @qinsoon Do you have any thoughts?

Copy link
Member

@qinsoon qinsoon Apr 15, 2024

Choose a reason for hiding this comment

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

I wonder if we should do things differently for Intel Macs vs Apple Silicon?

Maybe. But our support for intel macOS is minimal as well so this is not a big concern for now.


/// Strategy for performing mmap
///
/// This currently supports switching between different huge page allocation
Expand All @@ -77,7 +83,7 @@ pub enum MmapStrategy {
/// This function will not overwrite existing memory mapping, and it will result Err if there is an existing mapping.
#[allow(clippy::let_and_return)] // Zeroing is not neceesary for some OS/s
pub fn dzmmap_noreplace(start: Address, size: usize, strategy: MmapStrategy) -> Result<()> {
let prot = PROT_READ | PROT_WRITE | PROT_EXEC;
let prot = MMAP_PROT;
let flags = MMAP_FLAGS;
let ret = mmap_fixed(start, size, prot, flags, strategy);
// We do not need to explicitly zero for Linux (memory is guaranteed to be zeroed)
Expand Down