Skip to content

Commit d232b59

Browse files
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.
1 parent b3da8fa commit d232b59

File tree

2 files changed

+26
-0
lines changed

2 files changed

+26
-0
lines changed

Diff for: cores/arduino/main.cpp

+7
Original file line numberDiff line numberDiff line change
@@ -56,3 +56,10 @@ int main( void )
5656

5757
return 0;
5858
}
59+
60+
// Ensure our version of sbrk is used by forcing a reference to it here
61+
// (without this, an undefined reference only occurs later when linking
62+
// newlib, and the dummy from libgloss/nosys will be used instead).
63+
// This variable will be optimized away later.
64+
extern "C" void *_sbrk(int);
65+
void * force_reference_to_sbrk = (void*)&_sbrk;

Diff for: cores/arduino/sbrk.c

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
#include <samd.h>
2+
#include <stddef.h>
3+
#include <errno.h>
4+
5+
void *_sbrk(int incr)
6+
{
7+
extern char end; /* End of global variables, defined by the linker */
8+
static char *brkval = &end ;
9+
char *prev_brkval = brkval;
10+
11+
if (brkval + incr > (char *)__get_MSP()) {
12+
/* Heap and stack collision */
13+
errno = ENOMEM;
14+
return (void*) -1;
15+
}
16+
17+
brkval += incr ;
18+
return (void*) prev_brkval;
19+
}

0 commit comments

Comments
 (0)