diff --git a/.github/workflows/build-and-test.yml b/.github/workflows/build-and-test.yml index 92d47e28..12ab3168 100644 --- a/.github/workflows/build-and-test.yml +++ b/.github/workflows/build-and-test.yml @@ -20,6 +20,8 @@ jobs: - uses: actions/checkout@v6 - name: Build run: CC=gcc-${{ matrix.version }} CXX=g++-${{ matrix.version }} make test + - name: Build with MADV_GUARD_INSTALL guard pages + run: make clean && CC=gcc-${{ matrix.version }} CXX=g++-${{ matrix.version }} make test CONFIG_GUARD_PAGES_USE_MADVISE=true build-ubuntu-clang: runs-on: ubuntu-latest strategy: @@ -35,6 +37,8 @@ jobs: sudo update-alternatives --install /usr/bin/clang clang /usr/bin/clang-${{ matrix.version }} 100 - name: Build run: CC=clang CXX=clang++ make test + - name: Build with MADV_GUARD_INSTALL guard pages + run: make clean && CC=clang CXX=clang++ make test CONFIG_GUARD_PAGES_USE_MADVISE=true build-musl: runs-on: ubuntu-latest container: @@ -45,6 +49,8 @@ jobs: run: apk update && apk add build-base python3 - name: Build run: make test + - name: Build with MADV_GUARD_INSTALL guard pages + run: make clean && make test CONFIG_GUARD_PAGES_USE_MADVISE=true build-ubuntu-gcc-aarch64: runs-on: ubuntu-latest container: diff --git a/Makefile b/Makefile index 7ad20e87..f0ad3b8b 100644 --- a/Makefile +++ b/Makefile @@ -93,6 +93,10 @@ ifeq (,$(filter $(CONFIG_LABEL_MEMORY),true false)) $(error CONFIG_LABEL_MEMORY must be true or false) endif +ifeq (,$(filter $(CONFIG_GUARD_PAGES_USE_MADVISE),true false)) + $(error CONFIG_GUARD_PAGES_USE_MADVISE must be true or false) +endif + CPPFLAGS += \ -DCONFIG_SEAL_METADATA=$(CONFIG_SEAL_METADATA) \ -DZERO_ON_FREE=$(CONFIG_ZERO_ON_FREE) \ @@ -113,7 +117,8 @@ CPPFLAGS += \ -DN_ARENA=$(CONFIG_N_ARENA) \ -DCONFIG_STATS=$(CONFIG_STATS) \ -DCONFIG_SELF_INIT=$(CONFIG_SELF_INIT) \ - -DCONFIG_LABEL_MEMORY=$(CONFIG_LABEL_MEMORY) + -DCONFIG_LABEL_MEMORY=$(CONFIG_LABEL_MEMORY) \ + -DGUARD_PAGES_USE_MADVISE=$(CONFIG_GUARD_PAGES_USE_MADVISE) $(OUT)/libhardened_malloc$(SUFFIX).so: $(OBJECTS) | $(OUT) $(CC) $(CFLAGS) $(LDFLAGS) -shared $^ $(LDLIBS) -o $@ @@ -125,7 +130,7 @@ $(OUT)/chacha.o: chacha.c chacha.h util.h $(CONFIG_FILE) | $(OUT) $(COMPILE.c) $(OUTPUT_OPTION) $< $(OUT)/h_malloc.o: h_malloc.c include/h_malloc.h mutex.h memory.h pages.h random.h util.h $(CONFIG_FILE) | $(OUT) $(COMPILE.c) $(OUTPUT_OPTION) $< -$(OUT)/memory.o: memory.c memory.h util.h $(CONFIG_FILE) | $(OUT) +$(OUT)/memory.o: memory.c memory.h pages.h util.h $(CONFIG_FILE) | $(OUT) $(COMPILE.c) $(OUTPUT_OPTION) $< $(OUT)/new.o: new.cc include/h_malloc.h util.h $(CONFIG_FILE) | $(OUT) $(COMPILE.cc) $(OUTPUT_OPTION) $< diff --git a/README.md b/README.md index 7ffe1142..cf1a2731 100644 --- a/README.md +++ b/README.md @@ -279,6 +279,21 @@ The following boolean configuration options are available: hardware, which may become drastically lower in the future. Whether or not this feature is enabled, the metadata is all contained within an isolated memory region with high entropy random guard regions around it. +* `CONFIG_GUARD_PAGES_USE_MADVISE`: `true` or `false` (default) to control + whether the guard regions for large allocations are created with + `MADV_GUARD_INSTALL` (Linux 6.13+) inside a single read-write mapping instead + of as separate `PROT_NONE` mappings. This keeps each large allocation to a + single VMA instead of three, which substantially reduces VMA pressure from + large allocations and from the region quarantine. Kernel support is probed at + runtime and the allocator falls back to the `PROT_NONE` scheme when it is + unavailable or when a guard cannot be installed (e.g. on locked mappings); + the probe trusts the `madvise` return value, so the feature must be validated + on a real kernel (qemu-user, for example, silently ignores the advice). It is + off by default because the guard bytes and quarantined regions are then + accounted as committed memory (resident memory and the total address space + are unchanged, since the pages are never backed or are freed by the guard + installation), which can be a problem under strict overcommit + (`vm.overcommit_memory=2`). The following integer configuration options are available: diff --git a/config/default.mk b/config/default.mk index d8f03a78..38955564 100644 --- a/config/default.mk +++ b/config/default.mk @@ -22,3 +22,4 @@ CONFIG_N_ARENA := 4 CONFIG_STATS := false CONFIG_SELF_INIT := true CONFIG_LABEL_MEMORY := false +CONFIG_GUARD_PAGES_USE_MADVISE := false diff --git a/config/light.mk b/config/light.mk index da8676d9..92197a30 100644 --- a/config/light.mk +++ b/config/light.mk @@ -22,3 +22,4 @@ CONFIG_N_ARENA := 4 CONFIG_STATS := false CONFIG_SELF_INIT := true CONFIG_LABEL_MEMORY := false +CONFIG_GUARD_PAGES_USE_MADVISE := false diff --git a/h_malloc.c b/h_malloc.c index be66d964..f1a444ec 100644 --- a/h_malloc.c +++ b/h_malloc.c @@ -1019,10 +1019,13 @@ static void regions_quarantine_deallocate_pages(void *p, size_t size, size_t gua return; } - if (unlikely(memory_map_fixed(p, size))) { + if (unlikely(memory_guard_or_protnone(p, size))) { if (unlikely(memory_purge(p, size))) { memset(p, 0, size); } + } else if (GUARD_PAGES_USE_MADVISE && memory_guard_install_supported()) { + // name the whole region to avoid splitting the single VMA + memory_set_name((char *)p - guard_size, size + guard_size * 2, "malloc large quarantine"); } else { memory_set_name(p, size, "malloc large quarantine"); } @@ -1596,7 +1599,7 @@ EXPORT void *h_realloc(void *old, size_t size) { // in-place shrink if (size < old_size) { void *new_end = (char *)old + size; - if (memory_map_fixed(new_end, old_guard_size)) { + if (memory_guard_or_protnone(new_end, old_guard_size)) { thread_seal_metadata(); return NULL; } @@ -1619,6 +1622,7 @@ EXPORT void *h_realloc(void *old, size_t size) { #ifdef HAVE_COMPATIBLE_MREMAP static const bool vma_merging_reliable = false; + // not updated for the madvise guard scheme; revisit guard handling before enabling if (vma_merging_reliable) { // in-place growth void *guard_end = (char *)old + old_size + old_guard_size; diff --git a/memory.c b/memory.c index 45826f38..07e4a0bb 100644 --- a/memory.c +++ b/memory.c @@ -1,9 +1,14 @@ #include +#include #include #include +#ifndef MADV_GUARD_INSTALL +#define MADV_GUARD_INSTALL 102 +#endif + #ifndef PR_SET_VMA #define PR_SET_VMA 0x53564d41 #endif @@ -13,6 +18,7 @@ #endif #include "memory.h" +#include "pages.h" #include "util.h" static void *memory_map_prot(size_t size, int prot) { @@ -30,6 +36,10 @@ void *memory_map(size_t size) { return memory_map_prot(size, PROT_NONE); } +void *memory_map_rw(size_t size) { + return memory_map_prot(size, PROT_READ|PROT_WRITE); +} + #ifdef HAS_ARM_MTE // Note that PROT_MTE can't be cleared via mprotect void *memory_map_mte(size_t size) { @@ -117,6 +127,56 @@ bool memory_purge(void *ptr, size_t size) { return ret; } +// 0 = unknown, 1 = supported, -1 = unsupported/disabled +static atomic_int guard_install_state; + +// EINVAL means the mapping is locked, so reset the state to trigger a re-probe +bool memory_guard_install(void *ptr, size_t size) { + int saved_errno = errno; + if (likely(madvise(ptr, size, MADV_GUARD_INSTALL) == 0)) { + return false; + } + if (errno == EINVAL) { + int expected = 1; + atomic_compare_exchange_strong_explicit(&guard_install_state, &expected, 0, + memory_order_relaxed, memory_order_relaxed); + } + errno = saved_errno; + return true; +} + +bool memory_guard_install_supported(void) { + int s = atomic_load_explicit(&guard_install_state, memory_order_relaxed); + if (likely(s)) { + return s > 0; + } + int saved_errno = errno; + void *p = mmap(NULL, PAGE_SIZE, PROT_READ|PROT_WRITE, MAP_ANONYMOUS|MAP_PRIVATE, -1, 0); + if (p == MAP_FAILED) { + errno = saved_errno; + return false; + } + // EINVAL on a fresh mapping means no kernel support or mlockall(MCL_FUTURE) + s = madvise(p, PAGE_SIZE, MADV_GUARD_INSTALL) == 0 ? 1 : (errno == EINVAL ? -1 : 0); + munmap(p, PAGE_SIZE); + errno = saved_errno; + if (s) { + int expected = 0; + if (!atomic_compare_exchange_strong_explicit(&guard_install_state, &expected, s, + memory_order_relaxed, memory_order_relaxed)) { + s = expected; + } + } + return s > 0; +} + +bool memory_guard_or_protnone(void *ptr, size_t size) { + if (GUARD_PAGES_USE_MADVISE && memory_guard_install_supported()) { + return memory_guard_install(ptr, size) && memory_map_fixed(ptr, size); + } + return memory_map_fixed(ptr, size); +} + bool memory_set_name(UNUSED void *ptr, UNUSED size_t size, UNUSED const char *name) { if (CONFIG_LABEL_MEMORY) { return prctl(PR_SET_VMA, PR_SET_VMA_ANON_NAME, ptr, size, name); diff --git a/memory.h b/memory.h index b1156386..df605e0b 100644 --- a/memory.h +++ b/memory.h @@ -12,9 +12,17 @@ #define CONFIG_LABEL_MEMORY false #endif +#ifndef GUARD_PAGES_USE_MADVISE +#define GUARD_PAGES_USE_MADVISE false +#endif + int get_metadata_key(void); void *memory_map(size_t size); +void *memory_map_rw(size_t size); +bool memory_guard_install(void *ptr, size_t size); +bool memory_guard_install_supported(void); +bool memory_guard_or_protnone(void *ptr, size_t size); #ifdef HAS_ARM_MTE void *memory_map_mte(size_t size); #endif diff --git a/pages.c b/pages.c index 26045ce3..aa4400bf 100644 --- a/pages.c +++ b/pages.c @@ -16,6 +16,21 @@ void *allocate_pages(size_t usable_size, size_t guard_size, bool unprotect, cons errno = ENOMEM; return NULL; } + // MADV_GUARD_INSTALL needs page-aligned ranges + if (GUARD_PAGES_USE_MADVISE && unprotect && (usable_size & (PAGE_SIZE - 1)) == 0 && + memory_guard_install_supported()) { + void *guarded = memory_map_rw(real_size); + if (likely(guarded != NULL)) { + memory_set_name(guarded, real_size, name); + void *usable = (char *)guarded + guard_size; + if (!guard_size || (!memory_guard_install(guarded, guard_size) && + !memory_guard_install((char *)usable + usable_size, guard_size))) { + return usable; + } + memory_unmap(guarded, real_size); + } + } + void *real = memory_map(real_size); if (unlikely(real == NULL)) { return NULL; @@ -48,38 +63,49 @@ void *allocate_pages_aligned(size_t usable_size, size_t alignment, size_t guard_ return NULL; } - void *real = memory_map(real_alloc_size); - if (unlikely(real == NULL)) { - return NULL; - } - memory_set_name(real, real_alloc_size, name); + bool use_madvise = GUARD_PAGES_USE_MADVISE && memory_guard_install_supported(); - void *usable = (char *)real + guard_size; + for (;;) { + void *real = use_madvise ? memory_map_rw(real_alloc_size) : memory_map(real_alloc_size); + if (unlikely(real == NULL)) { + return NULL; + } + memory_set_name(real, real_alloc_size, name); - size_t lead_size = align((uintptr_t)usable, alignment) - (uintptr_t)usable; - size_t trail_size = alloc_size - lead_size - usable_size; - void *base = (char *)usable + lead_size; + void *usable = (char *)real + guard_size; - if (unlikely(memory_protect_rw(base, usable_size))) { - memory_unmap(real, real_alloc_size); - return NULL; - } + size_t lead_size = align((uintptr_t)usable, alignment) - (uintptr_t)usable; + size_t trail_size = alloc_size - lead_size - usable_size; + void *base = (char *)usable + lead_size; - if (lead_size) { - if (unlikely(memory_unmap(real, lead_size))) { + if (!use_madvise && unlikely(memory_protect_rw(base, usable_size))) { memory_unmap(real, real_alloc_size); return NULL; } - } - if (trail_size) { - if (unlikely(memory_unmap((char *)base + usable_size + guard_size, trail_size))) { - memory_unmap(real, real_alloc_size); - return NULL; + if (lead_size) { + if (unlikely(memory_unmap(real, lead_size))) { + memory_unmap(real, real_alloc_size); + return NULL; + } } - } - return base; + if (trail_size) { + if (unlikely(memory_unmap((char *)base + usable_size + guard_size, trail_size))) { + memory_unmap(real, real_alloc_size); + return NULL; + } + } + + if (use_madvise && guard_size && (unlikely(memory_guard_install((char *)base - guard_size, guard_size)) || + unlikely(memory_guard_install((char *)base + usable_size, guard_size)))) { + memory_unmap((char *)base - guard_size, usable_size + guard_size * 2); + use_madvise = false; + continue; + } + + return base; + } } void deallocate_pages(void *usable, size_t usable_size, size_t guard_size) { diff --git a/test/Makefile b/test/Makefile index fb680a4d..8e9c2173 100644 --- a/test/Makefile +++ b/test/Makefile @@ -55,8 +55,13 @@ EXECUTABLES := \ uninitialized_malloc_usable_size \ overflow_large_1_byte \ overflow_large_8_byte \ + overflow_large_aligned \ + overflow_realloc_shrink_large \ overflow_small_1_byte \ overflow_small_8_byte \ + underflow_large_1_byte \ + underflow_large_aligned \ + read_after_realloc_shrink_large \ string_overflow \ delete_type_size_mismatch \ aligned_sized_delete_small \ diff --git a/test/overflow_large_aligned.c b/test/overflow_large_aligned.c new file mode 100644 index 00000000..9863bb53 --- /dev/null +++ b/test/overflow_large_aligned.c @@ -0,0 +1,15 @@ +#include +#include + +#include "test_util.h" + +OPTNONE int main(void) { + void *p = NULL; + if (posix_memalign(&p, 64 * 1024, 256 * 1024)) { + return 1; + } + size_t size = malloc_usable_size(p); + *((char *)p + size) = 0; + free(p); + return 0; +} diff --git a/test/overflow_realloc_shrink_large.c b/test/overflow_realloc_shrink_large.c new file mode 100644 index 00000000..d6996337 --- /dev/null +++ b/test/overflow_realloc_shrink_large.c @@ -0,0 +1,21 @@ +#include +#include +#include + +#include "test_util.h" + +OPTNONE int main(void) { + char *p = malloc(4 * 1024 * 1024); + if (!p) { + return 1; + } + memset(p, 'a', 4 * 1024 * 1024); + char *q = realloc(p, 1024 * 1024); + if (!q) { + return 1; + } + size_t size = malloc_usable_size(q); + *(q + size) = 0; + free(q); + return 0; +} diff --git a/test/read_after_realloc_shrink_large.c b/test/read_after_realloc_shrink_large.c new file mode 100644 index 00000000..73fc1497 --- /dev/null +++ b/test/read_after_realloc_shrink_large.c @@ -0,0 +1,17 @@ +#include +#include + +#include "test_util.h" + +OPTNONE int main(void) { + char *p = malloc(4 * 1024 * 1024); + if (!p) { + return 1; + } + memset(p, 'a', 4 * 1024 * 1024); + if (!realloc(p, 1024 * 1024)) { + return 1; + } + // the discarded tail is guarded or quarantined, never still readable + return p[2 * 1024 * 1024] == 'a' ? 0 : 1; +} diff --git a/test/test_smc.py b/test/test_smc.py index c8a350d9..fd6b899c 100644 --- a/test/test_smc.py +++ b/test/test_smc.py @@ -95,6 +95,31 @@ def test_overflow_large_8_byte(self): "overflow_large_8_byte") self.assertEqual(returncode, -11) + def test_overflow_large_aligned(self): + _stdout, _stderr, returncode = self.run_test( + "overflow_large_aligned") + self.assertEqual(returncode, -11) + + def test_overflow_realloc_shrink_large(self): + _stdout, _stderr, returncode = self.run_test( + "overflow_realloc_shrink_large") + self.assertEqual(returncode, -11) + + def test_underflow_large_1_byte(self): + _stdout, _stderr, returncode = self.run_test( + "underflow_large_1_byte") + self.assertEqual(returncode, -11) + + def test_underflow_large_aligned(self): + _stdout, _stderr, returncode = self.run_test( + "underflow_large_aligned") + self.assertEqual(returncode, -11) + + def test_read_after_realloc_shrink_large(self): + _stdout, _stderr, returncode = self.run_test( + "read_after_realloc_shrink_large") + self.assertEqual(returncode, -11) + def test_overflow_small_1_byte(self): _stdout, stderr, returncode = self.run_test( "overflow_small_1_byte") diff --git a/test/underflow_large_1_byte.c b/test/underflow_large_1_byte.c new file mode 100644 index 00000000..1a72425c --- /dev/null +++ b/test/underflow_large_1_byte.c @@ -0,0 +1,13 @@ +#include + +#include "test_util.h" + +OPTNONE int main(void) { + char *p = malloc(256 * 1024); + if (!p) { + return 1; + } + *(p - 1) = 0; + free(p); + return 0; +} diff --git a/test/underflow_large_aligned.c b/test/underflow_large_aligned.c new file mode 100644 index 00000000..7949d470 --- /dev/null +++ b/test/underflow_large_aligned.c @@ -0,0 +1,13 @@ +#include + +#include "test_util.h" + +OPTNONE int main(void) { + void *p = NULL; + if (posix_memalign(&p, 64 * 1024, 256 * 1024)) { + return 1; + } + *((char *)p - 1) = 0; + free(p); + return 0; +}