ArduinoCore-sam icon indicating copy to clipboard operation
ArduinoCore-sam copied to clipboard

_sbrk has no bounds checking

Open WestfW opened this issue 3 years ago • 2 comments

_sbrk() in syscalls_sam3.c, does not contain any checks to see if the code is trying to acquired non-existent memory. This can lead malloc() and thus new to return pointers to either non-existent memory, or memory that belongs to the stack.

See https://forum.arduino.cc/t/ungraceful-handling-of-heap-depletion/1023509

WestfW avatar Aug 19 '22 06:08 WestfW

I recently ran into this for samd too, which I think is similar to sam. I have a local fix I should create a pullrequest for, but got distracted with other things before I could finish it. I'll try to see if I can push it out maybe next week, though I probably won't have time to port it to sam as well.

matthijskooijman avatar Aug 19 '22 08:08 matthijskooijman

I just published my fix for samd at https://github.com/arduino/ArduinoCore-samd/pull/681, see that PR for a more detailed analysis (also check the commit messages) and a test sketch.

Looking at the sam core, it seems that the problem is essentially the same (sbrk() never failing), but where samd uses gcc's libnosys to provide it, the sam core (like @WestfW already mentioned) just contains a broken _sbrk() itself: https://github.com/arduino/ArduinoCore-sam/blob/790ff2c852bf159787a9966bddee4d9f55352d15/cores/arduino/syscalls_sam3.c#L64

The fix is the same, though, just add a proper _sbrk() implementation. At first glance the samd version from my PR would be suitable as-is, except samd uses end and sam uses _end to signal the end of global variables.

I won't have time to prepare a PR for this, but would be happy to review and assist if anyone else does so.

matthijskooijman avatar Sep 06 '22 14:09 matthijskooijman