Skip to content

Commit e0e3f9e

Browse files
SkewedZeppelinSkewedZeppelin
committed
perform size checks on memcpy/memmove/memset
- memset is disabled for now as it causes hangs - underlying functions were copied from isoalloc, licensed Apache-2.0 - credit Chris Rohlf for memcpy/memset - credit David Carlier for memmove Signed-off-by: Tavi <tavi@divested.dev>
1 parent 4fe9018 commit e0e3f9e

22 files changed

+373
-8
lines changed

‎Android.bp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ common_cflags = [
2828
"-DN_ARENA=1",
2929
"-DCONFIG_STATS=true",
3030
"-DCONFIG_SELF_INIT=false",
31+
"-DCONFIG_BLOCK_OPS_CHECK_SIZE=false",
3132
]
3233

3334
cc_defaults {

‎CREDITS

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ h_malloc.c open-addressed hash table (regions_grow, regions_insert, regions_find
2323
ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF
2424
OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
2525

26+
h_malloc.c block operations (h_memcpy_real, h_memmove_real, h_memset_real):
27+
28+
Copyright (C) 2022, 2023 struct <chris.rohlf@gmail.com>
29+
Copyright (C) 2023 David Carlier <devnexen@gmail.com>
30+
Apache-2.0
31+
2632
libdivide:
2733

2834
Copyright (C) 2010 - 2019 ridiculous_fish, <libdivide@ridiculousfish.com>

‎Makefile

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,10 @@ ifeq (,$(filter $(CONFIG_SELF_INIT),true false))
8989
$(error CONFIG_SELF_INIT must be true or false)
9090
endif
9191

92+
ifeq (,$(filter $(CONFIG_BLOCK_OPS_CHECK_SIZE),true false))
93+
$(error CONFIG_BLOCK_OPS_CHECK_SIZE must be true or false)
94+
endif
95+
9296
CPPFLAGS += \
9397
-DCONFIG_SEAL_METADATA=$(CONFIG_SEAL_METADATA) \
9498
-DZERO_ON_FREE=$(CONFIG_ZERO_ON_FREE) \
@@ -108,7 +112,8 @@ CPPFLAGS += \
108112
-DCONFIG_CLASS_REGION_SIZE=$(CONFIG_CLASS_REGION_SIZE) \
109113
-DN_ARENA=$(CONFIG_N_ARENA) \
110114
-DCONFIG_STATS=$(CONFIG_STATS) \
111-
-DCONFIG_SELF_INIT=$(CONFIG_SELF_INIT)
115+
-DCONFIG_SELF_INIT=$(CONFIG_SELF_INIT) \
116+
-DCONFIG_BLOCK_OPS_CHECK_SIZE=$(CONFIG_BLOCK_OPS_CHECK_SIZE)
112117

113118
$(OUT)/libhardened_malloc$(SUFFIX).so: $(OBJECTS) | $(OUT)
114119
$(CC) $(CFLAGS) $(LDFLAGS) -shared $^ $(LDLIBS) -o $@

‎README.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,10 @@ The following boolean configuration options are available:
276276
hardware, which may become drastically lower in the future. Whether or not
277277
this feature is enabled, the metadata is all contained within an isolated
278278
memory region with high entropy random guard regions around it.
279+
* `CONFIG_BLOCK_OPS_CHECK_SIZE`: `true` or `false` (default) to ensure length
280+
parameter of the memcpy/memmove/memset block operations are within
281+
approximate bounds to minimize buffer overflows. Note, memset override is
282+
currently disabled due to improper behavior.
279283

280284
The following integer configuration options are available:
281285

‎config/default.mk

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,4 @@ CONFIG_CLASS_REGION_SIZE := 34359738368 # 32GiB
2121
CONFIG_N_ARENA := 4
2222
CONFIG_STATS := false
2323
CONFIG_SELF_INIT := true
24+
CONFIG_BLOCK_OPS_CHECK_SIZE := false

‎config/light.mk

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,3 +21,4 @@ CONFIG_CLASS_REGION_SIZE := 34359738368 # 32GiB
2121
CONFIG_N_ARENA := 4
2222
CONFIG_STATS := false
2323
CONFIG_SELF_INIT := true
24+
CONFIG_BLOCK_OPS_CHECK_SIZE := false

‎h_malloc.c

Lines changed: 94 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -528,7 +528,7 @@ static void set_canary(UNUSED const struct slab_metadata *metadata, UNUSED void
528528
}
529529
#endif
530530

531-
memcpy((char *)p + size - canary_size, &metadata->canary_value, canary_size);
531+
h_memcpy_real((char *)p + size - canary_size, &metadata->canary_value, canary_size);
532532
#endif
533533
}
534534

@@ -541,7 +541,7 @@ static void check_canary(UNUSED const struct slab_metadata *metadata, UNUSED con
541541
#endif
542542

543543
u64 canary_value;
544-
memcpy(&canary_value, (const char *)p + size - canary_size, canary_size);
544+
h_memcpy_real(&canary_value, (const char *)p + size - canary_size, canary_size);
545545

546546
#ifdef HAS_ARM_MTE
547547
if (unlikely(canary_value == 0)) {
@@ -831,7 +831,7 @@ static inline void deallocate_small(void *p, const size_t *expected_size) {
831831
#endif
832832

833833
if (ZERO_ON_FREE && !skip_zero) {
834-
memset(p, 0, size - canary_size);
834+
h_memset_real(p, 0, size - canary_size);
835835
}
836836
}
837837

@@ -1502,7 +1502,7 @@ EXPORT void *h_calloc(size_t nmemb, size_t size) {
15021502
total_size = adjust_size_for_canary(total_size);
15031503
void *p = alloc(total_size);
15041504
if (!ZERO_ON_FREE && likely(p != NULL) && total_size && total_size <= max_slab_size_class) {
1505-
memset(p, 0, total_size - canary_size);
1505+
h_memset_real(p, 0, total_size - canary_size);
15061506
}
15071507
#ifdef HAS_ARM_MTE
15081508
// use an assert instead of adding a conditional to memset() above (freed memory is always
@@ -1624,7 +1624,7 @@ EXPORT void *h_realloc(void *old, size_t size) {
16241624
mutex_unlock(&ra->lock);
16251625

16261626
if (memory_remap_fixed(old, old_size, new, size)) {
1627-
memcpy(new, old, copy_size);
1627+
h_memcpy_real(new, old, copy_size);
16281628
deallocate_pages(old, old_size, old_guard_size);
16291629
} else {
16301630
memory_unmap((char *)old - old_guard_size, old_guard_size);
@@ -1646,7 +1646,7 @@ EXPORT void *h_realloc(void *old, size_t size) {
16461646
if (copy_size > 0 && copy_size <= max_slab_size_class) {
16471647
copy_size -= canary_size;
16481648
}
1649-
memcpy(new, old_orig, copy_size);
1649+
h_memcpy_real(new, old_orig, copy_size);
16501650
if (old_size <= max_slab_size_class) {
16511651
deallocate_small(old, NULL);
16521652
} else {
@@ -1874,6 +1874,94 @@ EXPORT size_t h_malloc_object_size_fast(const void *p) {
18741874
return SIZE_MAX;
18751875
}
18761876

1877+
inline void *h_memcpy_real(void *dst, const void *src, size_t len) {
1878+
#if CONFIG_BLOCK_OPS_CHECK_SIZE
1879+
char *p_dst = (char *)dst;
1880+
char const *p_src = (char const *)src;
1881+
1882+
while(len--) {
1883+
*p_dst++ = *p_src++;
1884+
}
1885+
1886+
return dst;
1887+
#else
1888+
return memcpy(dst, src, len);
1889+
#endif
1890+
}
1891+
1892+
inline void *h_memmove_real(void *dst, const void *src, size_t len) {
1893+
#if CONFIG_BLOCK_OPS_CHECK_SIZE
1894+
char *p_dst = (char *)dst;
1895+
char const *p_src = (char const *)src;
1896+
1897+
if(p_src < p_dst) {
1898+
p_dst += len;
1899+
p_src += len;
1900+
while(len--) {
1901+
*--p_dst = *--p_src;
1902+
}
1903+
} else {
1904+
dst = h_memcpy_real(dst, src, len);
1905+
}
1906+
1907+
return dst;
1908+
#else
1909+
return memmove(dst, src, len);
1910+
#endif
1911+
}
1912+
1913+
inline void *h_memset_real(void *dst, int value, size_t len) {
1914+
#if CONFIG_BLOCK_OPS_CHECK_SIZE
1915+
char *p_dst = (char *)dst;
1916+
1917+
while(len--) {
1918+
*p_dst++ = value;
1919+
}
1920+
1921+
return dst;
1922+
#else
1923+
return memset(dst, value, len);
1924+
#endif
1925+
}
1926+
1927+
#if CONFIG_BLOCK_OPS_CHECK_SIZE
1928+
EXPORT void *h_memcpy(void *dst, const void *src, size_t len) {
1929+
if(dst == src || len == 0) {
1930+
return dst;
1931+
}
1932+
if (len > malloc_object_size(src)) {
1933+
fatal_error("memcpy read overflow");
1934+
}
1935+
if (len > malloc_object_size(dst)) {
1936+
fatal_error("memcpy buffer overflow");
1937+
}
1938+
return h_memcpy_real(dst, src, len);
1939+
}
1940+
1941+
EXPORT void *h_memmove(void *dst, const void *src, size_t len) {
1942+
if(dst == src || len == 0) {
1943+
return dst;
1944+
}
1945+
if (len > malloc_object_size(src)) {
1946+
fatal_error("memmove read overflow");
1947+
}
1948+
if (len > malloc_object_size(dst)) {
1949+
fatal_error("memmove buffer overflow");
1950+
}
1951+
return h_memmove_real(dst, src, len);
1952+
}
1953+
1954+
EXPORT void *h_memset(void *dst, int value, size_t len) {
1955+
if(len == 0) {
1956+
return dst;
1957+
}
1958+
if (len > malloc_object_size(dst)) {
1959+
fatal_error("memset buffer overflow");
1960+
}
1961+
return h_memset_real(dst, value, len);
1962+
}
1963+
#endif
1964+
18771965
EXPORT int h_mallopt(UNUSED int param, UNUSED int value) {
18781966
#ifdef __ANDROID__
18791967
if (param == M_PURGE) {

‎include/h_malloc.h

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ extern "C" {
1515
#define h_realloc realloc
1616
#define h_aligned_alloc aligned_alloc
1717
#define h_free free
18+
#if CONFIG_BLOCK_OPS_CHECK_SIZE
19+
#define h_memcpy memcpy
20+
#define h_memmove memmove
21+
//#define h_memset memset
22+
#endif
1823

1924
#define h_posix_memalign posix_memalign
2025

@@ -54,6 +59,14 @@ __attribute__((alloc_size(2))) void *h_realloc(void *ptr, size_t size);
5459
__attribute__((malloc)) __attribute__((alloc_size(2))) __attribute__((alloc_align(1)))
5560
void *h_aligned_alloc(size_t alignment, size_t size);
5661
void h_free(void *ptr);
62+
void *h_memcpy_real(void *dst, const void *src, size_t len);
63+
void *h_memmove_real(void *dst, const void *src, size_t len);
64+
void *h_memset_real(void *dst, int value, size_t len);
65+
#if CONFIG_BLOCK_OPS_CHECK_SIZE
66+
void *h_memcpy(void *dst, const void *src, size_t len);
67+
void *h_memmove(void *dst, const void *src, size_t len);
68+
void *h_memset(void *dst, int value, size_t len);
69+
#endif
5770

5871
// POSIX
5972
int h_posix_memalign(void **memptr, size_t alignment, size_t size);

‎test/.gitignore

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,4 +41,15 @@ overflow_small_8_byte
4141
uninitialized_read_large
4242
uninitialized_read_small
4343
realloc_init
44+
memcpy_buffer_overflow
45+
memcpy_read_overflow
46+
memcpy_valid_same
47+
memcpy_valid_mismatched
48+
memmove_buffer_overflow
49+
memmove_read_overflow
50+
memmove_valid_same
51+
memmove_valid_mismatched
52+
memset_buffer_overflow
53+
memset_valid_same
54+
memset_valid_mismatched
4455
__pycache__/

‎test/Makefile

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,18 @@ EXECUTABLES := \
6767
invalid_malloc_object_size_small \
6868
invalid_malloc_object_size_small_quarantine \
6969
impossibly_large_malloc \
70-
realloc_init
70+
realloc_init \
71+
memcpy_buffer_overflow \
72+
memcpy_read_overflow \
73+
memcpy_valid_same \
74+
memcpy_valid_mismatched \
75+
memmove_buffer_overflow \
76+
memmove_read_overflow \
77+
memmove_valid_same \
78+
memmove_valid_mismatched \
79+
memset_buffer_overflow \
80+
memset_valid_same \
81+
memset_valid_mismatched
7182

7283
all: $(EXECUTABLES)
7384

‎test/memcpy_buffer_overflow.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#include <stdlib.h>
2+
#include <string.h>
3+
4+
#include "test_util.h"
5+
6+
OPTNONE int main(void) {
7+
char *firstbuffer = malloc(16);
8+
char *secondbuffer = malloc(32);
9+
if (!firstbuffer && !secondbuffer) {
10+
return 1;
11+
}
12+
memset(secondbuffer, 'a', 32);
13+
memcpy(firstbuffer, secondbuffer, 32);
14+
return 1;
15+
}

‎test/memcpy_read_overflow.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#include <stdlib.h>
2+
#include <string.h>
3+
4+
#include "test_util.h"
5+
6+
OPTNONE int main(void) {
7+
char *firstbuffer = malloc(32);
8+
char *secondbuffer = malloc(16);
9+
if (!firstbuffer && !secondbuffer) {
10+
return 1;
11+
}
12+
memset(secondbuffer, 'a', 16);
13+
memcpy(firstbuffer, secondbuffer, 32);
14+
return 1;
15+
}

‎test/memcpy_valid_mismatched.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#include <stdlib.h>
2+
#include <string.h>
3+
4+
#include "test_util.h"
5+
6+
OPTNONE int main(void) {
7+
char *firstbuffer = malloc(32);
8+
char *secondbuffer = malloc(16);
9+
if (!firstbuffer && !secondbuffer) {
10+
return 1;
11+
}
12+
memset(secondbuffer, 'a', 16);
13+
memcpy(firstbuffer, secondbuffer, 16);
14+
return 0;
15+
}

‎test/memcpy_valid_same.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#include <stdlib.h>
2+
#include <string.h>
3+
4+
#include "test_util.h"
5+
6+
OPTNONE int main(void) {
7+
char *firstbuffer = malloc(16);
8+
char *secondbuffer = malloc(16);
9+
if (!firstbuffer && !secondbuffer) {
10+
return 1;
11+
}
12+
memset(secondbuffer, 'a', 16);
13+
memcpy(firstbuffer, secondbuffer, 16);
14+
return 0;
15+
}

‎test/memmove_buffer_overflow.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#include <stdlib.h>
2+
#include <string.h>
3+
4+
#include "test_util.h"
5+
6+
OPTNONE int main(void) {
7+
char *firstbuffer = malloc(16);
8+
char *secondbuffer = malloc(32);
9+
if (!firstbuffer && !secondbuffer) {
10+
return 1;
11+
}
12+
memset(secondbuffer, 'a', 32);
13+
memmove(firstbuffer, secondbuffer, 32);
14+
return 1;
15+
}

‎test/memmove_read_overflow.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#include <stdlib.h>
2+
#include <string.h>
3+
4+
#include "test_util.h"
5+
6+
OPTNONE int main(void) {
7+
char *firstbuffer = malloc(32);
8+
char *secondbuffer = malloc(16);
9+
if (!firstbuffer && !secondbuffer) {
10+
return 1;
11+
}
12+
memset(secondbuffer, 'a', 16);
13+
memmove(firstbuffer, secondbuffer, 32);
14+
return 1;
15+
}

‎test/memmove_valid_mismatched.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
#include <stdlib.h>
2+
#include <string.h>
3+
4+
#include "test_util.h"
5+
6+
OPTNONE int main(void) {
7+
char *firstbuffer = malloc(32);
8+
char *secondbuffer = malloc(16);
9+
if (!firstbuffer && !secondbuffer) {
10+
return 1;
11+
}
12+
memset(secondbuffer, 'a', 16);
13+
memmove(firstbuffer, secondbuffer, 16);
14+
return 0;
15+
}

0 commit comments

Comments
 (0)