On 12.11.2018 19:03, Heinrich Schuchardt wrote:
On 11/12/18 7:56 AM, Simon Goldschmidt wrote:
On Mon, Nov 12, 2018 at 12:22 AM Heinrich Schuchardt <xypron.g...@gmx.de> wrote:
On 11/11/18 3:22 PM, Wolfgang Denk wrote:
Dear Andrea,
In message <20181109094615.gc9...@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.
That's why I proposed to use the already existing memory reservation
scheme 'lmb' (used in loading boot images).
In your case, 'board_lmb_reserve' should make sure the secure monitor
does not get overwritten.
The 'arch_lmb_reserve' function for arm already ensures U-Boot text,
heap and stack don't get overwritten. It could be improved to reserve
+1M to the current stack pointer where it does reserve +4K now.
If board_lmb_reserve() should be the solution I would prefer that not
each individual board calls board_lmb_reserve() but that some common
code is used to iterate over the memory reservations in the device tree.
I know that the efi loader has its own scheme of memory reservation. I
just thought it cleaner to stay with lmb for fs_load and tftp as lmb is
already used in image loading/booting.
But using the memory reservations from U-Boot device tree definitively
makes sense, thanks for the hint. This should probably be done for the
bootm code, too...
Simon
Cheers
Heinrich
I am working on a patch for the 'load' issue (which could be reused
for the tftp issue). There are some problems with the existing lmb
code though, which delayed me a bit. However, given that this doesn't
make it into the 2018.11 release, anyway, I figured some more days to
get it cleaner won't hurt...
Simon
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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
