On 11/12/18 7:56 AM, Simon Goldschmidt wrote: > On Mon, Nov 12, 2018 at 12:22 AM Heinrich Schuchardt <[email protected]> > wrote: >> >> On 11/11/18 3:22 PM, Wolfgang Denk wrote: >>> Dear Andrea, >>> >>> In message <[email protected]> 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. 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 >> [email protected] >> https://lists.denx.de/listinfo/u-boot > _______________________________________________ U-Boot mailing list [email protected] https://lists.denx.de/listinfo/u-boot
