search

arduino / ArduinoCore-sam

80 stars 107 forks source link

_sbrk has no bounds checking #138

Open WestfW opened 2 years ago

WestfW commented 2 years ago

_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

matthijskooijman commented 2 years ago

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 commented 2 years ago

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.