[RFC PATCH] malloc: Harden malloc by protecting chunk fd/bk pointers
Arjun Shankar
arjun@redhat.com
Wed Dec 17 11:50:33 GMT 2025
malloc is susceptible to "unsafe unlink" attacks that manipulate chunk
fd/bk pointers before exploiting glibc's unlink_chunk. See:
https://github.com/shellphish/how2heap/blob/master/glibc_2.41/unsafe_unlink.c
This commit hardens malloc against such attacks.
This is achieved by using PROTECT_PTR when storing fd and bk pointers in
memory, thus making metadata manipulation harder. The changes are
mostly mechanical. New setters and getters are defined for fd and bk
pointers, using PROTECT_PTR and REVEAL_PTR respectively. They are then
used for every access.
A new test is also added to verify that the hardening works (by
causing a SIGSEGV at an unlink_chunk pointer check during free).
---
I ran into this problem while evaluating some old internal tests that looked
like candidates for upstreaming. One of them turned out to be an old
version of an unsafe-unlink exploit that has since been updated to the state
it's in at the how2heap repository that I learned about from Florian. I
was mostly curious if there can be some quick, low-cost hardening that can
protect against such attacks and if I could learn a bit more about malloc
while writing it. Is this patch useful, or a start towards something useful
for glibc?
---
malloc/Makefile | 6 ++
malloc/malloc.c | 182 ++++++++++++++++++++++++++------------------
malloc/tst-unlink.c | 84 ++++++++++++++++++++
3 files changed, 197 insertions(+), 75 deletions(-)
create mode 100644 malloc/tst-unlink.c
diff --git a/malloc/Makefile b/malloc/Makefile
index 5b3436dfd6..1c5a16741b 100644
--- a/malloc/Makefile
+++ b/malloc/Makefile
@@ -69,6 +69,7 @@ tests := \
tst-safe-linking \
tst-tcfree1 tst-tcfree2 tst-tcfree3 tst-tcfree4 \
tst-trim1 \
+ tst-unlink \
tst-valloc \
# tests
@@ -119,6 +120,7 @@ tests-exclude-malloc-check = \
tst-mxfast \
tst-safe-linking \
tst-tcfree4 \
+ tst-unlink \
# tests-exclude-malloc-check
# Run all tests with MALLOC_CHECK_=3
@@ -164,6 +166,7 @@ tests-exclude-largetcache = \
tst-malloc-usable \
tst-malloc-usable-tunables \
tst-mallocstate \
+ tst-unlink \
# tests-exclude-largetcache
tests-malloc-largetcache = \
@@ -194,6 +197,7 @@ tests-exclude-mcheck = \
tst-memalign-3 \
tst-mxfast \
tst-safe-linking \
+ tst-unlink \
# tests-exclude-mcheck
tests-mcheck = $(filter-out $(tests-exclude-mcheck) $(tests-static), $(tests))
@@ -456,6 +460,8 @@ CPPFLAGS-malloc.c += -DUSE_TCACHE=1
CFLAGS-tst-tcfree3.c += -fno-builtin-malloc -fno-builtin-free
+CFLAGS-tst-unlink.c := -O0
+
sLIBdir := $(shell echo $(slibdir) | sed 's,lib\(\|64\)$$,\\\\$$LIB,')
$(objpfx)mtrace: mtrace.pl
diff --git a/malloc/malloc.c b/malloc/malloc.c
index 8dd7bbe4f4..6beab77315 100644
--- a/malloc/malloc.c
+++ b/malloc/malloc.c
@@ -1451,6 +1451,34 @@ checked_request2size (size_t req) __nonnull (1)
/* Set size at footer (only when chunk is not in use) */
#define set_foot(p, s) (((mchunkptr) ((char *) (p) + (s)))->mchunk_prev_size = (s))
+/* Safe-Linking:
+ Use the same strategy used to protect single-linked lists of Fast-Bins
+ and TCache to protect regular double-linked chunk lists as well. */
+
+static __always_inline void
+set_fd (mchunkptr p, void *fd)
+{
+ p->fd = PROTECT_PTR (&p->fd, fd);
+}
+
+static __always_inline void
+set_bk (mchunkptr p, void *bk)
+{
+ p->bk = PROTECT_PTR (&p->bk, bk);
+}
+
+static __always_inline mchunkptr
+get_fd (mchunkptr p)
+{
+ return REVEAL_PTR (p->fd);
+}
+
+static __always_inline mchunkptr
+get_bk (mchunkptr p)
+{
+ return REVEAL_PTR (p->bk);
+}
+
#pragma GCC poison mchunk_size
#pragma GCC poison mchunk_prev_size
@@ -1581,8 +1609,8 @@ typedef struct malloc_chunk *mbinptr;
#define next_bin(b) ((mbinptr) ((char *) (b) + (sizeof (mchunkptr) << 1)))
/* Reminders about list directionality within bins */
-#define first(b) ((b)->fd)
-#define last(b) ((b)->bk)
+#define first(b) (get_fd (b))
+#define last(b) (get_bk (b))
/*
Indexing
@@ -1663,14 +1691,14 @@ unlink_chunk (mstate av, mchunkptr p)
if (chunksize (p) != prev_size (next_chunk (p)))
malloc_printerr ("corrupted size vs. prev_size");
- mchunkptr fd = p->fd;
- mchunkptr bk = p->bk;
+ mchunkptr fd = get_fd (p);
+ mchunkptr bk = get_bk (p);
- if (__glibc_unlikely (fd->bk != p || bk->fd != p))
+ if (__glibc_unlikely (get_bk (fd) != p || get_fd (bk) != p))
malloc_printerr ("corrupted double-linked list");
- fd->bk = bk;
- bk->fd = fd;
+ set_bk (fd, bk);
+ set_fd (bk, fd);
if (!in_smallbin_range (chunksize_nomask (p)) && p->fd_nextsize != NULL)
{
if (p->fd_nextsize->bk_nextsize != p
@@ -2000,7 +2028,8 @@ malloc_init_state (mstate av)
for (i = 1; i < NBINS; ++i)
{
bin = bin_at (av, i);
- bin->fd = bin->bk = bin;
+ set_fd (bin, bin);
+ set_bk (bin, bin);
}
#if MORECORE_CONTIGUOUS
@@ -2187,8 +2216,8 @@ do_check_free_chunk (mstate av, mchunkptr p)
assert (next == av->top || inuse (next));
/* ... and has minimally sane links */
- assert (p->fd->bk == p);
- assert (p->bk->fd == p);
+ assert (get_bk (get_fd (p)) == p);
+ assert (get_fd (get_bk (p)) == p);
}
else /* markers are always of size SIZE_SZ */
assert (sz == SIZE_SZ);
@@ -2385,7 +2414,7 @@ do_check_malloc_state (mstate av)
assert (binbit);
}
- for (p = last (b); p != b; p = p->bk)
+ for (p = last (b); p != b; p = get_bk (p))
{
/* each chunk claims to be free */
do_check_free_chunk (av, p);
@@ -2397,8 +2426,8 @@ do_check_malloc_state (mstate av)
idx = bin_index (size);
assert (idx == i);
/* lists are sorted */
- assert (p->bk == b ||
- (unsigned long) chunksize (p->bk) >= (unsigned long) chunksize (p));
+ assert (get_bk (p) == b ||
+ (unsigned long) chunksize (get_bk (p)) >= (unsigned long) chunksize (p));
if (!in_smallbin_range (size))
{
@@ -4148,12 +4177,12 @@ _int_malloc (mstate av, size_t bytes)
if ((victim = last (bin)) != bin)
{
- bck = victim->bk;
- if (__glibc_unlikely (bck->fd != victim))
+ bck = get_bk (victim);
+ if (__glibc_unlikely (get_fd (bck) != victim))
malloc_printerr ("malloc(): smallbin double linked list corrupted");
set_inuse_bit_at_offset (victim, nb);
- bin->bk = bck;
- bck->fd = bin;
+ set_bk (bin, bck);
+ set_fd (bck, bin);
if (av != &main_arena)
set_non_main_arena (victim);
@@ -4175,12 +4204,12 @@ _int_malloc (mstate av, size_t bytes)
{
if (tc_victim != NULL)
{
- bck = tc_victim->bk;
+ bck = get_bk (tc_victim);
set_inuse_bit_at_offset (tc_victim, nb);
if (av != &main_arena)
set_non_main_arena (tc_victim);
- bin->bk = bck;
- bck->fd = bin;
+ set_bk (bin, bck);
+ set_fd (bck, bin);
tcache_put (tc_victim, tc_idx);
}
@@ -4237,9 +4266,9 @@ _int_malloc (mstate av, size_t bytes)
for (;; )
{
int iters = 0;
- while ((victim = unsorted_chunks (av)->bk) != unsorted_chunks (av))
+ while ((victim = get_bk (unsorted_chunks (av))) != unsorted_chunks (av))
{
- bck = victim->bk;
+ bck = get_bk (victim);
size = chunksize (victim);
mchunkptr next = chunk_at_offset (victim, size);
@@ -4251,8 +4280,8 @@ _int_malloc (mstate av, size_t bytes)
malloc_printerr ("malloc(): invalid next size (unsorted)");
if (__glibc_unlikely ((prev_size (next) & ~(SIZE_BITS)) != size))
malloc_printerr ("malloc(): mismatching next->prev_size (unsorted)");
- if (__glibc_unlikely (bck->fd != victim)
- || __glibc_unlikely (victim->fd != unsorted_chunks (av)))
+ if (__glibc_unlikely (get_fd (bck) != victim)
+ || __glibc_unlikely (get_fd (victim) != unsorted_chunks (av)))
malloc_printerr ("malloc(): unsorted double linked list corrupted");
if (__glibc_unlikely (prev_inuse (next)))
malloc_printerr ("malloc(): invalid next->prev_inuse (unsorted)");
@@ -4273,9 +4302,11 @@ _int_malloc (mstate av, size_t bytes)
/* split and reattach remainder */
remainder_size = size - nb;
remainder = chunk_at_offset (victim, nb);
- unsorted_chunks (av)->bk = unsorted_chunks (av)->fd = remainder;
+ set_bk (unsorted_chunks (av), remainder);
+ set_fd (unsorted_chunks (av), remainder);
av->last_remainder = remainder;
- remainder->bk = remainder->fd = unsorted_chunks (av);
+ set_bk (remainder, unsorted_chunks (av));
+ set_fd (remainder, unsorted_chunks (av));
if (!in_smallbin_range (remainder_size))
{
remainder->fd_nextsize = NULL;
@@ -4294,8 +4325,8 @@ _int_malloc (mstate av, size_t bytes)
}
/* remove from unsorted list */
- unsorted_chunks (av)->bk = bck;
- bck->fd = unsorted_chunks (av);
+ set_bk (unsorted_chunks (av), bck);
+ set_fd (bck, unsorted_chunks (av));
/* Take now instead of binning if exact fit */
@@ -4334,13 +4365,13 @@ _int_malloc (mstate av, size_t bytes)
{
victim_index = smallbin_index (size);
bck = bin_at (av, victim_index);
- fwd = bck->fd;
+ fwd = get_fd (bck);
}
else
{
victim_index = largebin_index (size);
bck = bin_at (av, victim_index);
- fwd = bck->fd;
+ fwd = get_fd (bck);
/* maintain large bins in sorted order */
if (fwd != bck)
@@ -4348,19 +4379,20 @@ _int_malloc (mstate av, size_t bytes)
/* Or with inuse bit to speed comparisons */
size |= PREV_INUSE;
/* if smaller than smallest, bypass loop below */
- assert (chunk_main_arena (bck->bk));
+ assert (chunk_main_arena (get_bk (bck)));
if ((unsigned long) (size)
- < (unsigned long) chunksize_nomask (bck->bk))
+ < (unsigned long) chunksize_nomask (get_bk (bck)))
{
fwd = bck;
- bck = bck->bk;
+ bck = get_bk (bck);
- if (__glibc_unlikely (fwd->fd->bk_nextsize->fd_nextsize != fwd->fd))
+ if (__glibc_unlikely (get_fd (fwd)->bk_nextsize->fd_nextsize != get_fd (fwd)))
malloc_printerr ("malloc(): largebin double linked list corrupted (nextsize)");
- victim->fd_nextsize = fwd->fd;
- victim->bk_nextsize = fwd->fd->bk_nextsize;
- fwd->fd->bk_nextsize = victim->bk_nextsize->fd_nextsize = victim;
+ victim->fd_nextsize = get_fd (fwd);
+ victim->bk_nextsize = get_fd (fwd)->bk_nextsize;
+ get_fd (fwd)->bk_nextsize = victim;
+ victim->bk_nextsize->fd_nextsize = victim;
}
else
{
@@ -4374,7 +4406,7 @@ _int_malloc (mstate av, size_t bytes)
if ((unsigned long) size
== (unsigned long) chunksize_nomask (fwd))
/* Always insert in the second position. */
- fwd = fwd->fd;
+ fwd = get_fd (fwd);
else
{
victim->fd_nextsize = fwd;
@@ -4384,8 +4416,8 @@ _int_malloc (mstate av, size_t bytes)
fwd->bk_nextsize = victim;
victim->bk_nextsize->fd_nextsize = victim;
}
- bck = fwd->bk;
- if (bck->fd != fwd)
+ bck = get_bk (fwd);
+ if (get_fd (bck) != fwd)
malloc_printerr ("malloc(): largebin double linked list corrupted (bk)");
}
}
@@ -4394,10 +4426,10 @@ _int_malloc (mstate av, size_t bytes)
}
mark_bin (av, victim_index);
- victim->bk = bck;
- victim->fd = fwd;
- fwd->bk = victim;
- bck->fd = victim;
+ set_bk (victim, bck);
+ set_fd (victim, fwd);
+ set_bk (fwd, victim);
+ set_fd (bck, victim);
#if USE_TCACHE
/* If we've processed as many chunks as we're allowed while
@@ -4447,8 +4479,8 @@ _int_malloc (mstate av, size_t bytes)
list does not have to be rerouted. */
if (victim != last (bin)
&& chunksize_nomask (victim)
- == chunksize_nomask (victim->fd))
- victim = victim->fd;
+ == chunksize_nomask (get_fd (victim)))
+ victim = get_fd (victim);
remainder_size = size - nb;
unlink_chunk (av, victim);
@@ -4467,13 +4499,13 @@ _int_malloc (mstate av, size_t bytes)
/* We cannot assume the unsorted list is empty and therefore
have to perform a complete insert here. */
bck = unsorted_chunks (av);
- fwd = bck->fd;
- if (__glibc_unlikely (fwd->bk != bck))
+ fwd = get_fd (bck);
+ if (__glibc_unlikely (get_bk (fwd) != bck))
malloc_printerr ("malloc(): corrupted unsorted chunks");
- remainder->bk = bck;
- remainder->fd = fwd;
- bck->fd = remainder;
- fwd->bk = remainder;
+ set_bk (remainder, bck);
+ set_fd (remainder, fwd);
+ set_fd (bck, remainder);
+ set_bk (fwd, remainder);
if (!in_smallbin_range (remainder_size))
{
remainder->fd_nextsize = NULL;
@@ -4571,13 +4603,13 @@ _int_malloc (mstate av, size_t bytes)
/* We cannot assume the unsorted list is empty and therefore
have to perform a complete insert here. */
bck = unsorted_chunks (av);
- fwd = bck->fd;
- if (__glibc_unlikely (fwd->bk != bck))
+ fwd = get_fd (bck);
+ if (__glibc_unlikely (get_bk (fwd) != bck))
malloc_printerr ("malloc(): corrupted unsorted chunks 2");
- remainder->bk = bck;
- remainder->fd = fwd;
- bck->fd = remainder;
- fwd->bk = remainder;
+ set_bk (remainder, bck);
+ set_fd (remainder, fwd);
+ set_fd (bck, remainder);
+ set_bk (fwd, remainder);
/* advertise as last remainder */
if (in_smallbin_range (nb))
@@ -4878,8 +4910,8 @@ _int_free_create_chunk (mstate av, mchunkptr p, INTERNAL_SIZE_T size,
This branch is first in the if-statement to help branch
prediction on consecutive adjacent frees. */
bck = unsorted_chunks (av);
- fwd = bck->fd;
- if (__glibc_unlikely (fwd->bk != bck))
+ fwd = get_fd (bck);
+ if (__glibc_unlikely (get_bk (fwd) != bck))
malloc_printerr ("free(): corrupted unsorted chunks");
p->fd_nextsize = NULL;
p->bk_nextsize = NULL;
@@ -4890,18 +4922,18 @@ _int_free_create_chunk (mstate av, mchunkptr p, INTERNAL_SIZE_T size,
don't pollute the unsorted bin. */
int chunk_index = smallbin_index (size);
bck = bin_at (av, chunk_index);
- fwd = bck->fd;
+ fwd = get_fd (bck);
- if (__glibc_unlikely (fwd->bk != bck))
+ if (__glibc_unlikely (get_bk (fwd) != bck))
malloc_printerr ("free(): chunks in smallbin corrupted");
mark_bin (av, chunk_index);
}
- p->bk = bck;
- p->fd = fwd;
- bck->fd = p;
- fwd->bk = p;
+ set_bk (p, bck);
+ set_fd (p, fwd);
+ set_fd (bck, p);
+ set_bk (fwd, p);
set_head(p, size | PREV_INUSE);
set_foot(p, size);
@@ -5036,9 +5068,9 @@ static void malloc_consolidate(mstate av)
} else
clear_inuse_bit_at_offset(nextchunk, 0);
- first_unsorted = unsorted_bin->fd;
- unsorted_bin->fd = p;
- first_unsorted->bk = p;
+ first_unsorted = get_fd (unsorted_bin);
+ set_fd (unsorted_bin, p);
+ set_bk (first_unsorted, p);
if (!in_smallbin_range (size)) {
p->fd_nextsize = NULL;
@@ -5046,8 +5078,8 @@ static void malloc_consolidate(mstate av)
}
set_head(p, size | PREV_INUSE);
- p->bk = unsorted_bin;
- p->fd = first_unsorted;
+ set_bk (p, unsorted_bin);
+ set_fd (p, first_unsorted);
set_foot(p, size);
}
@@ -5277,7 +5309,7 @@ mtrim (mstate av, size_t pad)
{
mbinptr bin = bin_at (av, i);
- for (mchunkptr p = last (bin); p != bin; p = p->bk)
+ for (mchunkptr p = last (bin); p != bin; p = get_bk (p))
{
INTERNAL_SIZE_T size = chunksize (p);
@@ -5411,7 +5443,7 @@ int_mallinfo (mstate av, struct mallinfo2 *m)
for (i = 1; i < NBINS; ++i)
{
b = bin_at (av, i);
- for (p = last (b); p != b; p = p->bk)
+ for (p = last (b); p != b; p = get_bk (p))
{
++nblocks;
avail += chunksize (p);
@@ -6025,7 +6057,7 @@ __malloc_info (int options, FILE *fp)
for (size_t i = 1; i < NBINS; ++i)
{
bin = bin_at (ar_ptr, i);
- r = bin->fd;
+ r = get_fd (bin);
sizes[NFASTBINS - 1 + i].from = ~((size_t) 0);
sizes[NFASTBINS - 1 + i].to = sizes[NFASTBINS - 1 + i].total
= sizes[NFASTBINS - 1 + i].count = 0;
@@ -6041,7 +6073,7 @@ __malloc_info (int options, FILE *fp)
sizes[NFASTBINS - 1 + i].to = MAX (sizes[NFASTBINS - 1 + i].to,
r_size);
- r = r->fd;
+ r = get_fd (r);
}
if (sizes[NFASTBINS - 1 + i].count == 0)
diff --git a/malloc/tst-unlink.c b/malloc/tst-unlink.c
new file mode 100644
index 0000000000..6721dd38f3
--- /dev/null
+++ b/malloc/tst-unlink.c
@@ -0,0 +1,84 @@
+/* Test malloc hardening against unsafe-unlink heap exploits.
+ Copyright (C) 2025 Free Software Foundation, Inc.
+ This file is part of the GNU C Library.
+
+ The GNU C Library is free software; you can redistribute it and/or
+ modify it under the terms of the GNU Lesser General Public
+ License as published by the Free Software Foundation; either
+ version 2.1 of the License, or (at your option) any later version.
+
+ The GNU C Library is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+ Lesser General Public License for more details.
+
+ You should have received a copy of the GNU Lesser General Public
+ License along with the GNU C Library; if not, see
+ <https://www.gnu.org/licenses/>. */
+
+#include <stddef.h>
+#include <stdlib.h>
+#include <signal.h>
+
+/* Useful definitions and macros that match the malloc implementation: */
+struct malloc_chunk
+{
+ /* Chunk header. */
+ size_t mchunk_prev_size;
+ size_t mchunk_size;
+
+ /* (in-use chunk) user data, OR (free chunk) metadata. */
+ struct malloc_chunk* fd;
+ struct malloc_chunk* bk;
+};
+typedef struct malloc_chunk * mchunkptr;
+#define CHUNK_HDR_SZ (2 * sizeof (size_t))
+#define chunk2mem(p) ((void *) ((char *)(p) + CHUNK_HDR_SZ))
+#define mem2chunk(m) ((mchunkptr) ((char *)(m) - CHUNK_HDR_SZ))
+#define PREV_INUSE 0x1
+#define clear_inuse_bit_at_offset(p, s) \
+ (((mchunkptr) (((char *) (p)) + (s)))->mchunk_size &= ~(PREV_INUSE))
+
+/* Large allocations to avoid fastbins/tcache interactions. */
+#define ALLOC_SIZE (1024 + 64)
+
+void * global_ptr;
+
+static int
+do_test (void)
+{
+ void *local_ptr;
+
+ global_ptr = malloc (ALLOC_SIZE);
+ local_ptr = malloc (ALLOC_SIZE);
+
+ /* Create a fake chunk inside the first block: */
+ mchunkptr fake_chunk = global_ptr;
+
+ /* 1. Set up the size so that it still ends at the old boundary. */
+ fake_chunk->mchunk_size = ALLOC_SIZE;
+
+ /* 2. Set up fd and bk pointers to point to before the *address* of the
+ global pointer in such a way that the value stored there (pointer back
+ to us) appears to be the 'bk' of some imaginary chunk that is our 'fd',
+ and vice versa. Pre-2.43 glibc used to store these pointers
+ unprotected. */
+ fake_chunk->fd = (mchunkptr) ((char *) &global_ptr
+ - offsetof (struct malloc_chunk, bk));
+ fake_chunk->bk = (mchunkptr) ((char *) &global_ptr
+ - offsetof (struct malloc_chunk, fd));
+
+ /* Adjust the second block's chunk metadata so that (1) its prev_size is
+ correct, and (2) its previous chunk appears to be unused. */
+ mchunkptr local_chunk = mem2chunk (local_ptr);
+ local_chunk->mchunk_prev_size = ALLOC_SIZE;
+ clear_inuse_bit_at_offset (local_chunk, 0);
+
+ /* This should SIGSEGV when unlink_chunk tries to reveal fd/bk. */
+ free (local_ptr);
+
+ return 0;
+}
+
+#define EXPECTED_SIGNAL SIGSEGV
+#include <support/test-driver.c>
--
2.52.0
More information about the Libc-alpha
mailing list