From f463cced59ff220542cafa2f7ced742e899df7d2 Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Fri, 8 Jul 2022 16:55:34 +0200 Subject: [PATCH 1/5] Replace _sbrk to make malloc fail when out of memory When allocating memory with malloc, this calls _sbrk() to expand the heap. The current implementation always succeeds, which makes malloc always succeed, potentially returning memory that overlaps the stack, or is even outside of the available RAM. This commit fixes this by replacing the _sbrk() function by one which fails rather than allocating memory too close to the stack (with configurable margin). = Background & history = The malloc implementation used by this core is provided by newlib, which is shipped along gcc in the toolchain package and is a libc implementation intended for embedded systems. It implements the entire user-facing API, but relies on a number of "syscalls", to be provided by the underlying system to do the real work. See https://sourceware.org/newlib/libc.html#Syscalls These syscalls mostly concern handling of processes, and files, which are not applicable here, so dummy implementations that simply returned failure were originally provided by syscalls.c in this core. In addition, the _sbrk() syscall *is* relevant, since it is used to expand the heap when malloc runs out of memory to (re)use. Originally, the samd core provided a very simple version that always succeeded, which was later changed to fail when trying to allocate beyond the end of memory (but would still happily overwrite the stack). In commit 93c6355 (Resolving syscalls/_sbrk issue), the syscalls were removed, and (among a lot of other flag changes) `--specs=nosys.specs` was passed to the linker. Unlike what you might think, this "nosys" does not instruct the linker to omit any system library, but it tells the linker that no syscalls will be provided and makes it include the libgloss "nosys" library. This "nosys" library also provides dummy syscall implementations that always fail, except for _sbrk(), which always succeeds (like the original samd implementation). So this left the samd core with a malloc that *always* succeeds without regard for the stack or the end of RAM. = Solution = This is fixed by adding a _sbrk() implmentation that does check the current stack location and fails allocation if that would overwrite the stack. This implementation of _sbrk() is based on the STM32 core, except with some different variable and type names and with the minimum stack size check omitted (since the linker scripts do not define such a minimum size): https://github.com/stm32duino/Arduino_Core_STM32/blob/616258269f7e5a2ef8b2f71bb2a55616b25d07db/libraries/SrcWrapper/src/syscalls.c#L29-L51 Because libgloss nosys defines its dummy _sbrk() as a weak symbol, it can be overridden by a non-weak symbol in the samd core. However, because the core is linked through an archive file, it will only be included in the link if there is an unresolved reference to it (or anything else in the same file, which is nothing) at the time the core is linked. Normally, this undefined reference to _sbrk() is only introduced when linking newlib at the end of the link, which is then satisfied by libgloss (which is linked after newlib) rather than the samd core. To ensure that the samd core version is included instead, an artificial unresolved reference is created in main.c (which *is* always included because it defines main). This ensures that sbrk.c is pulled into the link instead of the libgloss version. --- cores/arduino/main.cpp | 7 +++++++ cores/arduino/sbrk.c | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 cores/arduino/sbrk.c diff --git a/cores/arduino/main.cpp b/cores/arduino/main.cpp index 3ea8ad3b3..59e124e78 100644 --- a/cores/arduino/main.cpp +++ b/cores/arduino/main.cpp @@ -59,3 +59,10 @@ int main( void ) return 0; } + +// Ensure our version of sbrk is used by forcing a reference to it here +// (without this, an undefined reference only occurs later when linking +// newlib, and the dummy from libgloss/nosys will be used instead). +// This variable will be optimized away later. +extern "C" void *_sbrk(int); +void * force_reference_to_sbrk = (void*)&_sbrk; diff --git a/cores/arduino/sbrk.c b/cores/arduino/sbrk.c new file mode 100644 index 000000000..ae0f772de --- /dev/null +++ b/cores/arduino/sbrk.c @@ -0,0 +1,19 @@ +#include +#include +#include + +void *_sbrk(int incr) +{ + extern char end; /* End of global variables, defined by the linker */ + static char *brkval = &end ; + char *prev_brkval = brkval; + + if (brkval + incr > (char *)__get_MSP()) { + /* Heap and stack collision */ + errno = ENOMEM; + return (void*) -1; + } + + brkval += incr ; + return (void*) prev_brkval; +} From 750ca7133fe0711605d4e1e77314ea5478131f6a Mon Sep 17 00:00:00 2001 From: Matthijs Kooijman Date: Tue, 19 Jul 2022 13:18:18 +0200 Subject: [PATCH 2/5] Let malloc leave some extra space for the stack The _sbrk() implementation added previously refuses to overwrite the stack, but will still happily allocate all other memory, leaving none to expand the stack into. Since the stack can expand over the heap undetected, this changes sbrk() to leave a safety margin between the heap and the stack (at the time of the malloc/_sbrk call). The size of this margin can be configured (by the sketch) by changing the `__malloc_margin` global variable, it defaults to 64 bytes. This approach (both having a margin and making it configurable with the `__malloc_margin`) is copied from the avr-libc malloc implementation, see: https://onlinedocs.microchip.com/pr/GUID-317042D4-BCCE-4065-BB05-AC4312DBC2C4-en-US-2/index.html?GUID-27757112-8BE1-49C2-B023-CD94AD06C5E2 avr-libc uses a default margin of 32 bytes, but given the bigger integer and pointer size on ARM, this is doubled for this core. --- cores/arduino/sbrk.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cores/arduino/sbrk.c b/cores/arduino/sbrk.c index ae0f772de..e3d4d8a3b 100644 --- a/cores/arduino/sbrk.c +++ b/cores/arduino/sbrk.c @@ -2,13 +2,17 @@ #include #include +/* How much free space to keep between the stack and the heap by malloc. + * Can be changed by the application at any time. */ +size_t __malloc_margin = 64; + void *_sbrk(int incr) { extern char end; /* End of global variables, defined by the linker */ static char *brkval = &end ; char *prev_brkval = brkval; - if (brkval + incr > (char *)__get_MSP()) { + if (brkval + incr + __malloc_margin > (char *)__get_MSP()) { /* Heap and stack collision */ errno = ENOMEM; return (void*) -1; From afd2a67307c0631929c62a154c89ca17251cb7ca Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Fri, 16 Oct 2026 16:07:50 -0700 Subject: [PATCH 3/5] Add license header to sbrk.c Use the LGPL 2.1 block the other core files carry. Co-Authored-By: Claude Opus 5.5 (1M context) --- cores/arduino/sbrk.c | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/cores/arduino/sbrk.c b/cores/arduino/sbrk.c index e3d4d8a3b..3c4306c50 100644 --- a/cores/arduino/sbrk.c +++ b/cores/arduino/sbrk.c @@ -1,3 +1,19 @@ +/* + This 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. + + This 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 this library; if not, write to the Free Software + Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA +*/ + #include #include #include From 496fe3355fde147d00f7f1d653ef4e3a575b247b Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Fri, 16 Oct 2026 16:07:55 -0700 Subject: [PATCH 4/5] Let sbrk(0) succeed when the heap is near the stack sbrk(0) only reads the current break, so it should never fail. Before this, it returned -1 once the heap was within __malloc_margin of the stack, which breaks free-memory helpers that subtract sbrk(0) from a stack address. Co-Authored-By: Claude Opus 5.5 (1M context) --- cores/arduino/sbrk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/arduino/sbrk.c b/cores/arduino/sbrk.c index 3c4306c50..a5fb6522a 100644 --- a/cores/arduino/sbrk.c +++ b/cores/arduino/sbrk.c @@ -28,7 +28,7 @@ void *_sbrk(int incr) static char *brkval = &end ; char *prev_brkval = brkval; - if (brkval + incr + __malloc_margin > (char *)__get_MSP()) { + if (incr > 0 && brkval + incr + __malloc_margin > (char *)__get_MSP()) { /* Heap and stack collision */ errno = ENOMEM; return (void*) -1; From e310bcd43fc3a1fd08f99ef8b7666632c9f1020d Mon Sep 17 00:00:00 2001 From: Mikey Sklar Date: Fri, 16 Oct 2026 16:33:22 -0700 Subject: [PATCH 5/5] Include sam.h in sbrk.c so SAME51 boards build samd.h only covers SAMD parts. On SAME51 boards like the Feather M4 CAN it pulls in no device header, so __get_MSP is never declared and the link fails. sam.h picks the right header for every family, the same as cortex_handlers.c. The generated code for SAMD21 and SAMD51 is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) --- cores/arduino/sbrk.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cores/arduino/sbrk.c b/cores/arduino/sbrk.c index a5fb6522a..39c3868ae 100644 --- a/cores/arduino/sbrk.c +++ b/cores/arduino/sbrk.c @@ -14,7 +14,7 @@ Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA */ -#include +#include #include #include