From 55a7ad2d588c256dab573c715ba7d983d1861736 Mon Sep 17 00:00:00 2001 From: Matthew Parkinson Date: Mon, 27 Sep 2021 13:32:46 +0100 Subject: [PATCH] Introduce PalEnforceAccess The various Pals were given different meanings in CHECK_CLIENT and non-CHECK_CLIENT builds. This was because it is essential that in the CHECK_CLIENT builds access is prevented, when not requested. This PR separates the CHECK_CLIENT concept from how the Pal should be implemented. --- docs/PORTING.md | 51 +++++++++++++++++-------------- src/pal/pal_apple.h | 38 +++++++++++------------ src/pal/pal_bsd.h | 12 +++++--- src/pal/pal_bsd_aligned.h | 9 ++---- src/pal/pal_consts.h | 15 +++++++++ src/pal/pal_freebsd_kernel.h | 4 +-- src/pal/pal_posix.h | 59 +++++++++++++++++++----------------- src/pal/pal_windows.h | 4 +-- 8 files changed, 108 insertions(+), 84 deletions(-) diff --git a/docs/PORTING.md b/docs/PORTING.md index f6c4567..0f1f2f0 100644 --- a/docs/PORTING.md +++ b/docs/PORTING.md @@ -16,31 +16,37 @@ The PAL must implement the following methods: ``` Report a fatal error and exit. +The memory that snmalloc is supplied from the Pal should be in one of three +states + +* `using` +* `using readonly` +* `not using` + +Before accessing the memory for a read, `snmalloc` will change the state to +either `using` or `using readonly`, +and before a write by it will change the state to `using`. +If memory is not required any more, then `snmalloc` will change the state to +`not using`, and will ensure that it notifies the `Pal` again +before it every accesses that memory again. +The `not using` state allows the `Pal` to recycle the memory for other purposes. +If `PalEnforceAccess` is set to true, then accessing that has not been notified +correctly should trigger an exception/segfault. + +The state for a particular region of memory is set with ```c++ static void notify_not_using(void* p, size_t size) noexcept; -``` -Notify the system that the range of memory from `p` to `p` + `size` is no -longer in use, allowing the underlying physical pages to recycled for other -purposes. -```c++ template static void notify_using(void* p, size_t size) noexcept; -``` -Notify the system that the range of memory from `p` to `p` + `size` is now in use. -On systems that lazily provide physical memory to virtual mappings, this -function may not be required to do anything. -If the template parameter is set to `YesZero` then this function is also -responsible for ensuring that the newly requested memory is full of zeros. -```c++ static void notify_using_readonly(void* p, size_t size) noexcept; ``` -Notify the system that the range of memory from `p` to `p` + `size` is now in use -for read-only access. This is currently only requried on platforms that support -`LazyCommit`. -On systems that lazily provide physical memory to virtual mappings, this -function may not be required to do anything. +These functions notify the system that the range of memory from `p` to `p` + +`size` is in the relevant state. + +If the template parameter is set to `YesZero` then `notify_using` must ensure +the range is full of zeros. ```c++ template @@ -54,18 +60,17 @@ efficient to request that the operating system provides background-zeroed pages, rather than zeroing them synchronously in this call ```c++ -template +template static void* reserve_aligned(size_t size) noexcept; static void* reserve(size_t size) noexcept; ``` All platforms should provide `reserve` and can optionally provide `reserve_aligned` if the underlying system can provide strongly aligned memory regions. -If the system guarantees only page alignment, implement only the second. The Pal is -free to overallocate based on the platform's desire and snmalloc -will find suitably aligned blocks inside the region. `reserve` should -not commit memory as snmalloc will commit the range of memory it requires of what -is returned. +If the system guarantees only page alignment, implement only the second. `snmalloc` will +overallocate to ensure it can find suitably aligned blocks inside the region. +`reserve` should consider memory initially as `not_using`, and `snmalloc` will notify when it +needs the range of memory. If the system provides strong alignment, implement the first to return memory at the desired alignment. If providing the first, then the `Pal` should also diff --git a/src/pal/pal_apple.h b/src/pal/pal_apple.h index a36e10c..14fb9f0 100644 --- a/src/pal/pal_apple.h +++ b/src/pal/pal_apple.h @@ -113,7 +113,7 @@ namespace snmalloc { SNMALLOC_ASSERT(is_aligned_block(p, size)); -# if defined(SNMALLOC_CHECK_CLIENT) && !defined(NDEBUG) +# if !defined(NDEBUG) memset(p, 0x5a, size); # endif @@ -126,12 +126,13 @@ namespace snmalloc while (madvise(p, size, MADV_FREE_REUSABLE) == -1 && errno == EAGAIN) ; -# ifdef SNMALLOC_CHECK_CLIENT - // This must occur after `MADV_FREE_REUSABLE`. - // - // `mach_vm_protect` is observably slower in benchmarks. - mprotect(p, size, PROT_NONE); -# endif + if constexpr (PalEnforceAccess) + { + // This must occur after `MADV_FREE_REUSABLE`. + // + // `mach_vm_protect` is observably slower in benchmarks. + mprotect(p, size, PROT_NONE); + } } /** @@ -180,12 +181,13 @@ namespace snmalloc } } -# ifdef SNMALLOC_CHECK_CLIENT - // Mark pages as writable for `madvise` below. - // - // `mach_vm_protect` is observably slower in benchmarks. - mprotect(p, size, PROT_READ | PROT_WRITE); -# endif + if constexpr (PalEnforceAccess) + { + // Mark pages as writable for `madvise` below. + // + // `mach_vm_protect` is observably slower in benchmarks. + mprotect(p, size, PROT_READ | PROT_WRITE); + } // `MADV_FREE_REUSE` can only be applied to writable pages, // otherwise it's an error. @@ -205,7 +207,7 @@ namespace snmalloc // Apple's `mmap` doesn't support user-specified alignment and only // guarantees mappings are aligned to the system page size, so we use // `mach_vm_map` instead. - template + template static void* reserve_aligned(size_t size) noexcept { SNMALLOC_ASSERT(bits::is_pow2(size)); @@ -219,11 +221,9 @@ namespace snmalloc // must be initialized to 0 or addr is interepreted as a lower-bound. mach_vm_address_t addr = 0; -# ifdef SNMALLOC_CHECK_CLIENT - vm_prot_t prot = committed ? VM_PROT_READ | VM_PROT_WRITE : VM_PROT_NONE; -# else - vm_prot_t prot = VM_PROT_READ | VM_PROT_WRITE; -# endif + vm_prot_t prot = (state_using || !PalEnforceAccess) ? + VM_PROT_READ | VM_PROT_WRITE : + VM_PROT_NONE; kern_return_t kr = mach_vm_map( mach_task_self(), diff --git a/src/pal/pal_bsd.h b/src/pal/pal_bsd.h index 38b3571..cf64244 100644 --- a/src/pal/pal_bsd.h +++ b/src/pal/pal_bsd.h @@ -34,14 +34,16 @@ namespace snmalloc static void notify_not_using(void* p, size_t size) noexcept { SNMALLOC_ASSERT(is_aligned_block(p, size)); - // Call this Pal to simulate the Windows decommit in CI. -#if defined(SNMALLOC_CHECK_CLIENT) && !defined(NDEBUG) + +#if !defined(NDEBUG) memset(p, 0x5a, size); #endif madvise(p, size, MADV_FREE); -#ifdef SNMALLOC_CHECK_CLIENT - mprotect(p, size, PROT_NONE); -#endif + + if constexpr (PalEnforceAccess) + { + mprotect(p, size, PROT_NONE); + } } }; } // namespace snmalloc diff --git a/src/pal/pal_bsd_aligned.h b/src/pal/pal_bsd_aligned.h index 592b842..7f1c1ea 100644 --- a/src/pal/pal_bsd_aligned.h +++ b/src/pal/pal_bsd_aligned.h @@ -28,7 +28,7 @@ namespace snmalloc /** * Reserve memory at a specific alignment. */ - template + template static void* reserve_aligned(size_t size) noexcept { // Alignment must be a power of 2. @@ -37,11 +37,8 @@ namespace snmalloc int log2align = static_cast(bits::next_pow2_bits(size)); -#ifdef SNMALLOC_CHECK_CLIENT - auto prot = committed ? PROT_READ | PROT_WRITE : PROT_NONE; -#else - auto prot = PROT_READ | PROT_WRITE; -#endif + auto prot = + state_using || !PalEnforceAccess ? PROT_READ | PROT_WRITE : PROT_NONE; void* p = mmap( nullptr, diff --git a/src/pal/pal_consts.h b/src/pal/pal_consts.h index 84823fe..0cb9e81 100644 --- a/src/pal/pal_consts.h +++ b/src/pal/pal_consts.h @@ -6,6 +6,21 @@ namespace snmalloc { + /** + * Pal implementations should query this flag to see whether they + * are allowed to optimise memory access, or that they must provide + * exceptions/segfaults if accesses do not obey the + * - using + * - using_readonly + * - not_using + * model. + */ +#ifdef SNMALLOC_CHECK_CLIENT + static constexpr bool PalEnforceAccess = true; +#else + static constexpr bool PalEnforceAccess = false; +#endif + /** * Flags in a bitfield of optional features that a PAL may support. These * should be set in the PAL's `pal_features` static constexpr field. diff --git a/src/pal/pal_freebsd_kernel.h b/src/pal/pal_freebsd_kernel.h index 084b38b..72f8487 100644 --- a/src/pal/pal_freebsd_kernel.h +++ b/src/pal/pal_freebsd_kernel.h @@ -59,7 +59,7 @@ namespace snmalloc ::bzero(p, size); } - template + template static void* reserve_aligned(size_t size) noexcept { SNMALLOC_ASSERT(bits::is_pow2(size)); @@ -80,7 +80,7 @@ namespace snmalloc { return nullptr; } - if (committed) + if (state_using) { if ( kmem_back(kernel_object, addr, size, M_ZERO | M_WAITOK) != diff --git a/src/pal/pal_posix.h b/src/pal/pal_posix.h index ff4cd52..55d96a5 100644 --- a/src/pal/pal_posix.h +++ b/src/pal/pal_posix.h @@ -178,17 +178,21 @@ namespace snmalloc static void notify_not_using(void* p, size_t size) noexcept { SNMALLOC_ASSERT(is_aligned_block(p, size)); -#ifdef SNMALLOC_CHECK_CLIENT - // Fill memory so that when we switch the pages back on we don't make - // assumptions on the content. -# if !defined(NDEBUG) - memset(p, 0x5a, size); -# endif - mprotect(p, size, PROT_NONE); -#else - UNUSED(p); - UNUSED(size); + + if constexpr (PalEnforceAccess) + { +#if !defined(NDEBUG) + // Fill memory so that when we switch the pages back on we don't make + // assumptions on the content. + memset(p, 0x5a, size); #endif + mprotect(p, size, PROT_NONE); + } + else + { + UNUSED(p); + UNUSED(size); + } } /** @@ -204,12 +208,13 @@ namespace snmalloc SNMALLOC_ASSERT( is_aligned_block(p, size) || (zero_mem == NoZero)); -#ifdef SNMALLOC_CHECK_CLIENT - mprotect(p, size, PROT_READ | PROT_WRITE); -#else - UNUSED(p); - UNUSED(size); -#endif + if constexpr (PalEnforceAccess) + mprotect(p, size, PROT_READ | PROT_WRITE); + else + { + UNUSED(p); + UNUSED(size); + } if constexpr (zero_mem == YesZero) zero(p, size); @@ -225,12 +230,13 @@ namespace snmalloc { SNMALLOC_ASSERT(is_aligned_block(p, size)); -#ifdef SNMALLOC_CHECK_CLIENT - mprotect(p, size, PROT_READ); -#else - UNUSED(p); - UNUSED(size); -#endif + if constexpr (PalEnforceAccess) + mprotect(p, size, PROT_READ); + else + { + UNUSED(p); + UNUSED(size); + } } /** @@ -286,11 +292,10 @@ namespace snmalloc */ static void* reserve(size_t size) noexcept { -#ifdef SNMALLOC_CHECK_CLIENT - auto prot = PROT_NONE; -#else - auto prot = PROT_READ | PROT_WRITE; -#endif + // If enforcing access, map pages initially as None, and then + // add permissions as required. Otherwise, immediately give all + // access as this is the most efficient to implement. + auto prot = PalEnforceAccess ? PROT_NONE : PROT_READ | PROT_WRITE; void* p = mmap( nullptr, diff --git a/src/pal/pal_windows.h b/src/pal/pal_windows.h index 86286d0..425810b 100644 --- a/src/pal/pal_windows.h +++ b/src/pal/pal_windows.h @@ -158,7 +158,7 @@ namespace snmalloc } # ifdef PLATFORM_HAS_VIRTUALALLOC2 - template + template static void* reserve_aligned(size_t size) noexcept { SNMALLOC_ASSERT(bits::is_pow2(size)); @@ -166,7 +166,7 @@ namespace snmalloc DWORD flags = MEM_RESERVE; - if (committed) + if (state_using) flags |= MEM_COMMIT; // If we're on Windows 10 or newer, we can use the VirtualAlloc2