On 11/11/18 3:22 PM, Wolfgang Denk wrote:
Dear Andrea,
In message 20181109094615.GC9586@lambda.inversepath.com you wrote:
Exactly, merely checking RAM size is not sufficient. The specific memory layout would need to be accounted for which means understanding where the stack and heap are located, their direction of growth and to ensure that the loaded payload can never overwrite them along with all other U-Boot data segments.
This is pretty easy. On all architectures I'm aware of the stack has the lowest location in memory, and is growing downward.
This is not easy given that the stack and heap size I think can only be guessed and not precisely limited, additionally board configurations have the ability to set arbitrary stack, relocation and load addresses which complicates things even further in understanding exactly how the memory layout is set.
I think this is not that complicated. At least in standard U-Boot (not speaking for SPL) it should be sufficient to check the current stack pointer (which is easy to read) and take this a upper limit of available/allowed memory. If we add some reasonable safety margin (say, 1 MB or so) we should be really safe.
Unfortunately this does not hold true. E.g. the Odroid-C2 has the secure monitor in the middle of the RAM. You would not want to overwrite those addresses.
For a board with a device tree all reserved memory areas should be secured against overwriting.
Best regards
Heinrich
Additionally, your patch checks the loaded file's size without taking the load address into account. So unless I read that wrong, your check is only valid for 'addr == 0'.
The approach is also not appliccable to networ boot; with TFTP we don't know the image size in advance.
Eventyally the boundary checking should be done where the image content actually gets copied to memory.
Best regards,
Wolfgang Denk