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.

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

Make jemalloc bound returned pointers to their usable size #2147

Merged
merged 5 commits into from
Nov 6, 2024
Merged

Make jemalloc bound returned pointers to their usable size #2147

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e5d6d65
mrs: Add a feature to make MRS apply bounds to returned pointers
markjdb Jul 3, 2024
ced6f9e
jemalloc: Avoid adding bounds unnecessarily
markjdb Jul 10, 2024
fa6c193
jemalloc: Clamp in malloc_usable_size() only if a tag is present
markjdb Jul 10, 2024
0ae431a
jemalloc: Bound pointers to the usable size
markjdb Jul 10, 2024
17ddd1f
Merge branch 'dev' into dev-jemalloc-bounds
markjdb Nov 6, 2024
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
24 changes: 14 additions & 10 deletions contrib/jemalloc/src/jemalloc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2273,7 +2273,13 @@
JEMALLOC_ALWAYS_INLINE int
imalloc(static_opts_t *sopts, dynamic_opts_t *dopts) {
int ret;
size_t size;

#ifdef __CHERI_PURE_CAPABILITY__
/*
* Ask for the usable size so that we can bound the returned pointer.
*/
sopts->usize = true;
#endif

/*
* CHERI: Rounding of allocation size occurs in imalloc_body()
Expand Down Expand Up @@ -2301,9 +2307,7 @@
ret = imalloc_body(sopts, dopts, tsd);
}
if (ret == 0) {
/* overflow causes imalloc_body to return ENOMEM */
(void)compute_size_with_overflow(1, dopts, &size);
*dopts->result = BOUND_PTR(*dopts->result, size);
*dopts->result = BOUND_PTR(*dopts->result, dopts->usize);
}
return ret;
}
Expand Down Expand Up @@ -2428,7 +2432,7 @@
LOG("core.malloc.exit", "result: %p", ret);

/* Fastpath success */
return BOUND_PTR(ret, size);
return BOUND_PTR(ret, sz_index2size(ind));

Check failure on line 2435 in contrib/jemalloc/src/jemalloc.c

View workflow job for this annotation

GitHub Actions / Style Checker 

parentheses required on return
}

return malloc_default(size);
Expand Down Expand Up @@ -2791,7 +2795,7 @@
check_entry_exit_locking(tsdn);

LOG("core.realloc.exit", "result: %p", ret);
return (BOUND_PTR(ret, size));
return (BOUND_PTR(ret, sz_s2u(size)));
}

JEMALLOC_NOINLINE
Expand Down Expand Up @@ -3325,7 +3329,7 @@
check_entry_exit_locking(tsd_tsdn(tsd));

LOG("core.rallocx.exit", "result: %p", p);
return BOUND_PTR(p, size);
return BOUND_PTR(p, sz_s2u(size));

Check failure on line 3332 in contrib/jemalloc/src/jemalloc.c

View workflow job for this annotation

GitHub Actions / Style Checker 

parentheses required on return
label_oom:
if (config_xmalloc && unlikely(opt_xmalloc)) {
malloc_write("<jemalloc>: Error in rallocx(): out of memory\n");
Expand Down Expand Up @@ -3770,7 +3774,7 @@
ret = isalloc(tsdn, ptr);
}
#ifdef __CHERI_PURE_CAPABILITY__
if (ret != 0) {
if (ret != 0 && cheri_gettag(ptr)) {
ret = MIN(ret, cheri_getlen(ptr));
}
#endif
Expand Down Expand Up @@ -3848,7 +3852,7 @@
if (rsize != NULL) {
*rsize = isalloc(tsdn_fetch(), p);
}
*ptr = BOUND_PTR(p, size);
*ptr = p;
return ALLOCM_SUCCESS;
}

Expand All @@ -3873,7 +3877,7 @@
} else {
void *p = je_rallocx(*ptr, size+extra, flags);
if (p != NULL) {
*ptr = BOUND_PTR(p, size+extra);
*ptr = p;
ret = ALLOCM_SUCCESS;
} else {
ret = ALLOCM_ERR_OOM;
Expand Down
103 changes: 90 additions & 13 deletions lib/libc/stdlib/malloc/mrs/mrs.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,10 @@
"_RUNTIME_REVOCATION_SYNC_REVOKE"
#define MALLOC_REVOKE_ASYNC_ENV \
"_RUNTIME_REVOCATION_ASYNC_REVOKE"
#define MALLOC_BOUND_CHERI_POINTERS \
"_RUNTIME_BOUND_CHERI_POINTERS"
#define MALLOC_NOBOUND_CHERI_POINTERS \
"_RUNTIME_NOBOUND_CHERI_POINTERS"

/*
* Different allocators give their strong symbols different names. Hide
Expand Down Expand Up @@ -296,6 +300,27 @@
static bool quarantining = true;
static bool revoke_every_free = false;
static bool revoke_async = false;
static bool bound_pointers = false;

/*
* Optionally apply strict bounds to pointers returned by malloc() and friends.
* This is technically UB since it means that the pointer passed to free() is
* not what we got from the allocator. However, it may be useful for
* demonstrating CHERI's spatial safety guarantees in demos and so on.
*
* The default behaviour lets the allocator provide pointers with bounds
* covering the usable size of the allocation, beyond the requested size, which
* is friendlier to realloc() loops. With revocation enabled, strict bounds
* would force a new allocation for each realloc() call, which can hurt quite a
* bit.
*/
static void *
mrs_bound_pointer(void *p, size_t size)
{
if (p != NULL && bound_pointers && size > 0)
p = cheri_setbounds(p, size);
Copy link
Member

Choose a reason for hiding this comment

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

I think we need to make sure that size is at least 1.

It's possible for an allocator to actually support zero sized allocations, but it needs a special pool so they can't be confused with an adjacent allocation manipulated to point to one past the end with bounds of 0. No current allocator does this (maybe out dlmalloc port does by accident).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I modified the routine to only bound the pointer if size > 0.

return (p);
}

struct mrs_descriptor_slab_entry {
void *ptr;
Expand Down Expand Up @@ -1283,6 +1308,11 @@
else if (getenv(MALLOC_REVOKE_ASYNC_ENV) != NULL)
revoke_async = true;
#endif

if (getenv(MALLOC_BOUND_CHERI_POINTERS) != NULL)
bound_pointers = true;
else if (getenv(MALLOC_NOBOUND_CHERI_POINTERS) != NULL)
bound_pointers = false;
}
if (!quarantining)
#if defined(PRINT_CAPREVOKE) || defined(PRINT_CAPREVOKE_MRS) || defined(PRINT_STATS)
Expand Down Expand Up @@ -1344,11 +1374,17 @@

/* mrs functions */

static void *
mrs_real_malloc(size_t size)
{
return (mrs_bound_pointer(REAL(malloc)(size), size));
}

static void *
mrs_malloc(size_t size)
{
if (!quarantining)
return (REAL(malloc)(size));
return (mrs_real_malloc(size));

/*mrs_debug_printf("mrs_malloc: called\n");*/

Expand All @@ -1366,9 +1402,9 @@
* of the currently supported allocators do.
*/
if (size < CAPREVOKE_BITMAP_ALIGNMENT)
allocated_region = REAL(malloc)(CAPREVOKE_BITMAP_ALIGNMENT);
allocated_region = mrs_real_malloc(CAPREVOKE_BITMAP_ALIGNMENT);
else
allocated_region = REAL(malloc)(size);
allocated_region = mrs_real_malloc(size);
if (allocated_region == NULL) {
MRS_UTRACE(UTRACE_MRS_MALLOC, NULL, size, 0,
allocated_region);
Expand All @@ -1388,13 +1424,19 @@
return (allocated_region);
}

static void *
mrs_real_calloc(size_t number, size_t size)
{
return (mrs_bound_pointer(REAL(calloc)(number, size), number * size));
}

void *
mrs_calloc(size_t number, size_t size)
{
size_t tmpsize;

if (!quarantining)
return (REAL(calloc)(number, size));
return (mrs_real_calloc(number, size));

/*
* This causes problems if our library is initialized before
Expand All @@ -1417,9 +1459,9 @@
*/
if (!__builtin_mul_overflow(number, size, &tmpsize) &&
tmpsize < CAPREVOKE_BITMAP_ALIGNMENT)
allocated_region = REAL(calloc)(1, CAPREVOKE_BITMAP_ALIGNMENT);
allocated_region = mrs_real_calloc(1, CAPREVOKE_BITMAP_ALIGNMENT);

Check warning on line 1462 in lib/libc/stdlib/malloc/mrs/mrs.c

View workflow job for this annotation

GitHub Actions / Style Checker 

line over 80 characters
else
allocated_region = REAL(calloc)(number, size);
allocated_region = mrs_real_calloc(number, size);
if (allocated_region == NULL) {
MRS_UTRACE(UTRACE_MRS_CALLOC, NULL, size, number,
allocated_region);
Expand All @@ -1438,11 +1480,22 @@
return (allocated_region);
}

static int
mrs_real_posix_memalign(void **ptr, size_t alignment, size_t size)
{
int ret;

ret = REAL(posix_memalign)(ptr, alignment, size);
if (ret == 0)
*ptr = mrs_bound_pointer(*ptr, size);
return (ret);
}

static int
mrs_posix_memalign(void **ptr, size_t alignment, size_t size)
{
if (!quarantining)
return (REAL(posix_memalign)(ptr, alignment, size));
return (mrs_real_posix_memalign(ptr, alignment, size));

mrs_debug_printf("mrs_posix_memalign: called ptr %p alignment %zu size %zu\n",
ptr, alignment, size);
Expand All @@ -1452,7 +1505,7 @@
if (alignment < CAPREVOKE_BITMAP_ALIGNMENT)
alignment = CAPREVOKE_BITMAP_ALIGNMENT;

int ret = REAL(posix_memalign)(ptr, alignment, size);
int ret = mrs_real_posix_memalign(ptr, alignment, size);
if (ret != 0) {
return (ret);
}
Expand All @@ -1467,11 +1520,17 @@
return (ret);
}

static void *
mrs_real_aligned_alloc(size_t alignment, size_t size)
{
return (mrs_bound_pointer(REAL(aligned_alloc)(alignment, size), size));
}

static void *
mrs_aligned_alloc(size_t alignment, size_t size)
{
if (!quarantining)
return (REAL(aligned_alloc)(alignment, size));
return (mrs_real_aligned_alloc(alignment, size));

mrs_debug_printf("mrs_aligned_alloc: called alignment %zu size %zu\n",
alignment, size);
Expand All @@ -1481,7 +1540,7 @@
if (alignment < CAPREVOKE_BITMAP_ALIGNMENT)
alignment = CAPREVOKE_BITMAP_ALIGNMENT;

void *allocated_region = REAL(aligned_alloc)(alignment, size);
void *allocated_region = mrs_real_aligned_alloc(alignment, size);
if (allocated_region == NULL) {
MRS_UTRACE(UTRACE_MRS_ALIGNED_ALLOC, NULL, size, alignment,
allocated_region);
Expand All @@ -1499,6 +1558,12 @@
return (allocated_region);
}

static void *
mrs_real_realloc(void *ptr, size_t size)
{
return (mrs_bound_pointer(REAL(realloc)(ptr, size), size));
}

/*
* Replace realloc with a malloc and free to avoid dangling pointers
* in case of in-place realloc that shrinks the buffer. If ptr is not
Expand All @@ -1509,7 +1574,7 @@
mrs_realloc(void *ptr, size_t size)
{
if (!quarantining)
return (REAL(realloc)(ptr, size));
return (mrs_real_realloc(ptr, size));

size_t old_size = cheri_getlen(ptr);
mrs_debug_printf("mrs_realloc: called ptr %p ptr size %zu new size %zu\n",
Expand Down Expand Up @@ -1583,14 +1648,20 @@
check_and_perform_flush(true);
}

static void *
mrs_real_mallocx(size_t size, int flags)
{
return (mrs_bound_pointer(REAL(mallocx)(size, flags), size));
}

void *
mrs_mallocx(size_t size, int flags)
{
size_t align = MALLOCX_ALIGN_GET(flags);
void *ret;

if (!quarantining)
return (REAL(mallocx)(size, flags));
return (mrs_real_mallocx(size, flags));

if (align <= CAPREVOKE_BITMAP_ALIGNMENT)
ret = mrs_malloc(size);
Expand All @@ -1605,17 +1676,23 @@
return (ret);
}

static void *
mrs_real_rallocx(void *ptr, size_t size, int flags)
{
return (mrs_bound_pointer(REAL(rallocx)(ptr, size, flags), size));
}

void *
mrs_rallocx(void *ptr, size_t size, int flags)
{
void *new_alloc;
size_t old_size;

if (!quarantining)
return (REAL(rallocx)(ptr, size, flags));
return (mrs_real_rallocx(ptr, size, flags));

old_size = cheri_getlen(ptr);

Check warning on line 1695 in lib/libc/stdlib/malloc/mrs/mrs.c

View workflow job for this annotation

GitHub Actions / Style Checker 

Missing Signed-off-by: line
mrs_debug_printf("%s: called ptr %p ptr size %zu new size %zu\n",
__func__, ptr, old_size, size);

Expand Down
Loading