[PATCH 1/6] malloc: define a maximum malloc size

[PATCH 1/6] malloc: define a maximum malloc size

[Ahmad Fatoum]

TLSF and dlmalloc each already enforce a maximum allocation size.

Let's define SZ_1G as common maximum allocation size in preparation
for aligning all allocators to observe it.

The intention is to make code easier to reason about and as extra
hardening against possible, yet undiscovered, allocator bugs.

Suggested-by: Richard Weinberger <richard@nod.at>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 include/malloc.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/malloc.h b/include/malloc.h
index 7bee03dab236..c38726342c08 100644
--- a/include/malloc.h
+++ b/include/malloc.h
@@ -5,6 +5,9 @@
 #include <linux/compiler.h>
 #include <types.h>
 
+#define MALLOC_SHIFT_MAX	30
+#define MALLOC_MAX_SIZE		(1UL << MALLOC_SHIFT_MAX)
+
 #if IN_PROPER
 void *malloc(size_t) __alloc_size(1);
 size_t malloc_usable_size(void *);
-- 
2.39.5

[PATCH 2/6] tlsf: fail allocations exceeding MALLOC_MAX_SIZE

[Ahmad Fatoum]

TLSF already enforces a maximum allocation size of SZ_1G on 32-bit systems
and a size of SZ_4G on 64-bit systems. There's no need to differentiate
between the two, let's settle on SZ_1G for both by unconditionally using
the newly introduced MALLOC_SHIFT_MAX macro.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/tlsf.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/common/tlsf.c b/common/tlsf.c
index 4cd90e150de2..146f1b7ebe81 100644
--- a/common/tlsf.c
+++ b/common/tlsf.c
@@ -46,15 +46,7 @@ enum tlsf_private
 	** blocks below that size into the 0th first-level list.
 	*/
 
-#if defined (TLSF_64BIT)
-	/*
-	** TODO: We can increase this to support larger sizes, at the expense
-	** of more overhead in the TLSF structure.
-	*/
-	FL_INDEX_MAX = 32,
-#else
-	FL_INDEX_MAX = 30,
-#endif
+	FL_INDEX_MAX = MALLOC_SHIFT_MAX,
 	SL_INDEX_COUNT = (1 << SL_INDEX_COUNT_LOG2),
 	FL_INDEX_SHIFT = (SL_INDEX_COUNT_LOG2 + ALIGN_SIZE_LOG2),
 	FL_INDEX_COUNT = (FL_INDEX_MAX - FL_INDEX_SHIFT + 1),
@@ -824,15 +816,9 @@ pool_t tlsf_add_pool(tlsf_t tlsf, void* mem, size_t bytes)
 
 	if (pool_bytes < block_size_min || pool_bytes > block_size_max)
 	{
-#if defined (TLSF_64BIT)
-		printf("tlsf_add_pool: Memory size must be between 0x%x and 0x%x00 bytes.\n",
-			(unsigned int)(pool_overhead + block_size_min),
-			(unsigned int)((pool_overhead + block_size_max) / 256));
-#else
 		printf("tlsf_add_pool: Memory size must be between %u and %u bytes.\n",
 			(unsigned int)(pool_overhead + block_size_min),
 			(unsigned int)(pool_overhead + block_size_max));
-#endif
 		return 0;
 	}
 
-- 
2.39.5

[PATCH 3/6] dlmalloc: fail allocations exceeding MALLOC_MAX_SIZE

[Ahmad Fatoum]

We already enforce a maximum allocation size of LONG_MAX in dlmalloc,
but this is larger than any reasonable allocation size we would have.

Let's reduce it to the new common maximum of SZ_1G.

While at it, enforce this limit for all size and alignment
parameters.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/dlmalloc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/common/dlmalloc.c b/common/dlmalloc.c
index 731c46b584b0..2b5723e127c4 100644
--- a/common/dlmalloc.c
+++ b/common/dlmalloc.c
@@ -1162,7 +1162,7 @@ void *dlmalloc(size_t bytes)
 
 	INTERNAL_SIZE_T nb;
 
-	if ((long) bytes < 0) {
+	if (bytes > MALLOC_MAX_SIZE) {
 		errno = ENOMEM;
 		return NULL;
 	}
@@ -1499,7 +1499,7 @@ void *dlrealloc(void *oldmem, size_t bytes)
 	}
 #endif
 
-	if ((long)bytes < 0) {
+	if (bytes > MALLOC_MAX_SIZE) {
 		errno = ENOMEM;
 		return NULL;
 	}
@@ -1668,7 +1668,7 @@ void *dlmemalign(size_t alignment, size_t bytes)
 	mchunkptr remainder;	/* spare room at end to split off */
 	long remainder_size;	/* its size */
 
-	if ((long) bytes < 0) {
+	if (bytes > MALLOC_MAX_SIZE || alignment > MALLOC_MAX_SIZE) {
 		errno = ENOMEM;
 		return NULL;
 	}
@@ -1753,7 +1753,7 @@ void *dlcalloc(size_t n, size_t elem_size)
 	mchunkptr oldtop = top;
 	INTERNAL_SIZE_T oldtopsize = chunksize(top);
 
-	if ((long)n < 0) {
+	if (sz > MALLOC_MAX_SIZE) {
 		errno = ENOMEM;
 		return NULL;
 	}
-- 
2.39.5

[PATCH 4/6] sandbox: libc_malloc: fail allocations exceeding MALLOC_MAX_SIZE

[Ahmad Fatoum]

The libc allocator is a bit unpredictable, because overcommit can
mean that big allocations succeed initially, only to OOM later.

Let's thus enforce a maximum allocation limit on the barebox side in
alignment with the bare metal allocators.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/sandbox/os/libc_malloc.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/arch/sandbox/os/libc_malloc.c b/arch/sandbox/os/libc_malloc.c
index e34156cd49e7..ac97fc37eee5 100644
--- a/arch/sandbox/os/libc_malloc.c
+++ b/arch/sandbox/os/libc_malloc.c
@@ -7,6 +7,8 @@
 #include <malloc.h>
 
 #define BAREBOX_ENOMEM 12
+#define BAREBOX_MALLOC_MAX_SIZE 0x40000000
+
 extern int barebox_errno;
 
 void barebox_malloc_stats(void)
@@ -15,7 +17,10 @@ void barebox_malloc_stats(void)
 
 void *barebox_memalign(size_t alignment, size_t bytes)
 {
-	void *mem = memalign(alignment, bytes);
+	void *mem = NULL;
+
+	if (alignment <= BAREBOX_MALLOC_MAX_SIZE && bytes <= BAREBOX_MALLOC_MAX_SIZE)
+		mem = memalign(alignment, bytes);
 	if (!mem)
 		barebox_errno = BAREBOX_ENOMEM;
 
@@ -25,7 +30,10 @@ void *barebox_memalign(size_t alignment, size_t bytes)
 void *barebox_malloc(size_t size)
 {
 
-	void *mem = malloc(size);
+	void *mem = NULL;
+
+	if (size <= BAREBOX_MALLOC_MAX_SIZE)
+		mem = malloc(size);
 	if (!mem)
 		barebox_errno = BAREBOX_ENOMEM;
 
@@ -44,7 +52,10 @@ void barebox_free(void *ptr)
 
 void *barebox_realloc(void *ptr, size_t size)
 {
-	void *mem = realloc(ptr, size);
+	void *mem = NULL;
+
+	if (size <= BAREBOX_MALLOC_MAX_SIZE)
+		mem = realloc(ptr, size);
 	if (!mem)
 		barebox_errno = BAREBOX_ENOMEM;
 
@@ -53,7 +64,12 @@ void *barebox_realloc(void *ptr, size_t size)
 
 void *barebox_calloc(size_t n, size_t elem_size)
 {
-	void *mem = calloc(n, elem_size);
+	size_t product;
+	void *mem = NULL;
+
+	if (!__builtin_add_overflow(n, elem_size, &product) &&
+	    product <= BAREBOX_MALLOC_MAX_SIZE)
+		mem = calloc(n, elem_size);
 	if (!mem)
 		barebox_errno = BAREBOX_ENOMEM;
 
-- 
2.39.5

[PATCH 5/6] dummy_malloc: fail allocations exceeding MALLOC_MAX_SIZE

[Ahmad Fatoum]

For uniformity with the proper allocators, let's observe the same
maximum allocation size they do.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 common/dummy_malloc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/common/dummy_malloc.c b/common/dummy_malloc.c
index 140ba314272a..4973f35650ab 100644
--- a/common/dummy_malloc.c
+++ b/common/dummy_malloc.c
@@ -11,8 +11,10 @@ void malloc_stats(void)
 
 void *memalign(size_t alignment, size_t bytes)
 {
-	void *mem = sbrk(bytes + alignment);
+	void *mem = NULL;
 
+	if (alignment <= MALLOC_MAX_SIZE && bytes <= MALLOC_MAX_SIZE)
+		mem = sbrk(bytes + alignment);
 	if (!mem) {
 		errno = ENOMEM;
 		return NULL;
-- 
2.39.5

[PATCH 6/6] test: self: malloc: adapt to addition of MALLOC_MAX_SIZE

[Ahmad Fatoum]

Now that we enforce a maximum allocation size, adapt the tests to
account for this.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 test/self/malloc.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/test/self/malloc.c b/test/self/malloc.c
index 0feec810185f..79a32b5d97c7 100644
--- a/test/self/malloc.c
+++ b/test/self/malloc.c
@@ -84,16 +84,17 @@ static void test_malloc(void)
 	p = expect_alloc_ok(malloc(1));
 	free(p);
 
-	if (mem_malloc_size) {
-		tmp = expect_alloc_fail(malloc(RELOC_HIDE(SIZE_MAX, 0)));
-		free(tmp);
+	p = expect_alloc_fail(malloc(RELOC_HIDE(SIZE_MAX, 0)));
+	free(p);
 
-		if (0xf0000000 > mem_malloc_size) {
-			tmp = expect_alloc_fail(malloc(RELOC_HIDE(0xf0000000, 0)));
-			free(tmp);
-		}
+	p = expect_alloc_fail(malloc(RELOC_HIDE(MALLOC_MAX_SIZE, 1)));
+	free(p);
+
+	if (mem_malloc_size) {
+		tmp = expect_alloc_fail(malloc(RELOC_HIDE(MALLOC_MAX_SIZE, -1)));
+		free(tmp);
 	} else {
-		skipped_tests += 2;
+		skipped_tests++;
 	}
 
 	free(realloc(NULL, 1));
@@ -110,16 +111,10 @@ static void test_malloc(void)
 		tmp = expect_alloc_fail(realloc(p, mem_malloc_size));
 		free(tmp);
 
-		if (0xf0000000 > mem_malloc_size) {
-			tmp = expect_alloc_fail(realloc(p, RELOC_HIDE(0xf0000000, 0)));
-			free(tmp);
-		}
-
-		tmp = expect_alloc_fail(realloc(p, RELOC_HIDE(SIZE_MAX, 0)));
+		tmp = expect_alloc_fail(realloc(p, RELOC_HIDE(MALLOC_MAX_SIZE, -1)));
 		free(tmp);
-
 	} else {
-		skipped_tests += 3;
+		skipped_tests += 2;
 	}
 
 	free(p);
-- 
2.39.5

Re: [PATCH 1/6] malloc: define a maximum malloc size

[Sascha Hauer]

On Thu, 20 Feb 2025 21:30:27 +0100, Ahmad Fatoum wrote:
> TLSF and dlmalloc each already enforce a maximum allocation size.
> 
> Let's define SZ_1G as common maximum allocation size in preparation
> for aligning all allocators to observe it.
> 
> The intention is to make code easier to reason about and as extra
> hardening against possible, yet undiscovered, allocator bugs.
> 
> [...]

Applied, thanks!

[1/6] malloc: define a maximum malloc size
      https://git.pengutronix.de/cgit/barebox/commit/?id=6279bd2d3a33 (link may not be stable)
[2/6] tlsf: fail allocations exceeding MALLOC_MAX_SIZE
      https://git.pengutronix.de/cgit/barebox/commit/?id=b5c4d87dfb62 (link may not be stable)
[3/6] dlmalloc: fail allocations exceeding MALLOC_MAX_SIZE
      https://git.pengutronix.de/cgit/barebox/commit/?id=122a8979fefb (link may not be stable)
[4/6] sandbox: libc_malloc: fail allocations exceeding MALLOC_MAX_SIZE
      https://git.pengutronix.de/cgit/barebox/commit/?id=8839004152f7 (link may not be stable)
[5/6] dummy_malloc: fail allocations exceeding MALLOC_MAX_SIZE
      https://git.pengutronix.de/cgit/barebox/commit/?id=8dcd7add5943 (link may not be stable)
[6/6] test: self: malloc: adapt to addition of MALLOC_MAX_SIZE
      https://git.pengutronix.de/cgit/barebox/commit/?id=cb77a6989e59 (link may not be stable)

Best regards,
-- 
Sascha Hauer <s.hauer@pengutronix.de>