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

macos: avoid mprotect checks since 11.2 #105

Merged
merged 1 commit into from
Oct 18, 2021
Merged

Conversation

carenas
Copy link
Contributor

@carenas carenas commented Feb 21, 2021

starting with macOS 11.2, the mprotect calls for RWX pages will fail at
least in Apple Silicon.

avoid the additional check that was originally added to workaround WX
implementations that lie about the permission of pages that were
allocated (ex: HardenedBSD) unless it has been specifically requested
(through a compile flag).

for macOS this allows the implementation provided for Apple Silicon in
#90 to use pthread_jit_write_protect_np() for the versions that
had that support to flip permissions in hardware and therefore avoid the
segfault so it is no longer needed.

fixes: #99

@@ -182,10 +182,12 @@ static SLJIT_INLINE void* alloc_chunk(sljit_uw size)
if (retval == MAP_FAILED)
return NULL;

#ifdef SLIJT_WX_OS_NEEDSCHEK
Copy link
Contributor Author

@carenas carenas Feb 21, 2021

Choose a reason for hiding this comment

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

HardenedBSD might be the only OS I am aware of that might have still an implementation of PAX that lies on their mmap() and need this, other than macOS (which also lies, but had already working code to flip permissions as needed since #90)

there are no OS especific checks where this could be set yet, neither I am sure if the flag could have a better name so pushing this as a draft to discuss further before merging.

has been tested in macOS 11.2.1 arm64e and x86_64 though and solves the problem in HEAD

Copy link

Choose a reason for hiding this comment

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

Just to suggest a possible alternative, the PROT_READ | PROT_WRITE | PROT_EXEC bit could be a macro that is set to PROT_READ | PROT_EXEC for M1, because those are the actually permissions for the initial mmap. Then the mprotect check should work fine, right?

(This is just based on my understanding of how this works, I have no system to test.)

there are no OS especific checks where this could be set yet, neither I am sure if the flag could have a better name so pushing this as a draft to discuss further before merging.

Alternatively could make this an #ifndef SLJIT_WX_DONT_CHECK that is defined in the M1 branch.

@zherczeg
Copy link
Owner

This patch is in draft state for half a year, do you intend to work on this?

@carenas
Copy link
Contributor Author

carenas commented Jun 15, 2021

This patch is in draft state for half a year, do you intend to work on this?

yes, but have to admit I am still unsure of the right way to define it since it is common to several OS and their oddities

Starting with macOS 11.2, mprotect calls for RWX pages will fail
in Apple Silicon, even if the page was granted permission and it was
requested the MAP_JIT flag, to better reflect the fact that the page
returned by mmap wasn't really RWX.

In macOS, there is an implementation for the executable allocator since
e87e1cc (macos: add BigSur support to execalloc (zherczeg#90), 2020-11-30) that
flips the bits as needed, so this extra safeward is no longer needed.

HardenedBSD seems to be the last implementation of PaX that still lies,
so restrict the code only to that platform.

Fixes: zherczeg#99
@carenas carenas marked this pull request as ready for review October 18, 2021 07:49
@zherczeg
Copy link
Owner

Is this the final version of the patch?

@carenas
Copy link
Contributor Author

carenas commented Oct 18, 2021

the original implementation was too broad anyway, and therefore took a while to test on all affected systems with all different combinations, but the only one that still needs it is HardenedBSD, and luckily PaX implementations had mostly realized that lying in an mmap call, wasn't very effective.

also; specifically for macOS, the way the code was segmented per architecture helped to make sure there is really no impact on the mid release API change with BigSur since all arm64 versions will be using MAP_JIT anyway, and therefore will fail in the mmap if using the hardened runtime and missing entitlements.

sorry for the delay

Copy link
Owner

@zherczeg zherczeg left a comment

Choose a reason for hiding this comment

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

LGTM

@zherczeg zherczeg merged commit d6a0fa6 into zherczeg:master Oct 18, 2021
@carenas carenas deleted the macos11_2 branch October 18, 2021 08:18
Torrekie added a commit to Torrekie/Procursus that referenced this pull request Jan 4, 2022
cmb69 pushed a commit to php/php-src that referenced this pull request Aug 31, 2022
This backports zherczeg/sljit#105. Relates to bug #80435, however, it doesn't solve the bus error on PHP 8.0, but PHP 8.1 builds fine now.

Closes GH-9279.
alexforster added a commit to alexforster/bpfjit-sys that referenced this pull request Aug 9, 2024
Includes a backport of the fix in zherczeg/sljit#105, and an update to bpfjit.c to fix calls to `sljit_create_compiler` and `sljit_free_code`.
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.

Attempts to mprotect() with MAP_JIT failing on Apple Silicon as of macOS 11.2
3 participants