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. 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
