[U-Boot] [PATCH v3 1/5] common: introduce maximum load size

[Rob Herring]

On Tue, Apr 15, 2014 at 4:59 PM, Wolfgang Denk <wd at denx.de> wrote:
> Dear Rob,
>
> In message <1397157488-8695-2-git-send-email-robherring2 at gmail.com> you wrote:
>>
>> Various commands that load images have no checks that a loaded image
>> does not exceed the available RAM space and will happily continue
>> overwriting u-boot or other RAM that should not be touched. Also,
>> some commands such as USB DFU or fastboot need to know the maximum
>> buffer size, but there is no common way to define this.
>
> You are mixing some pretty much unrelated things here, which is IMO
> not a good idea.  Limiting the size for load operations is one thing,
> but buffer size is something totally different.  These must not be
> mixed up.  Please keep in mind that there is (or at least should be)
> no need to load the whole file into a buffer. [Limitations of the
> implementation should be fixed rather than supported.]

That is true if you are loading files to write to storage, but if you
are loading them to boot like tftp, diskboot, fsload, fastboot, etc.
all do, then you should have some checks in place. Otherwise you can
just load things over RAM u-boot is using.

fastboot is loading images to boot just like other commands that use
loadaddr, but has the additional need to know the max size. Rather
than add yet another custom define, I was looking at how to solve this
generically. I considered perhaps this should be based on LMB/bootmap?
But LMB is more about what the OS can use rather than what memory is
available.


>> Introduce a global load_size and environment variable loadsize to
>> specify the size. The default is ~0UL which is effectively unlimited.
>
> I think this feature should be made optional.  I don;t think I want to
> have this on all systems.
>
>> +  loadsize   - Maximum load size for commands like "bootp",
>> +               "rarpboot", "tftpboot", "loadb" or "diskboot"
>
> Um... what makes these operations different from any other operations
> that read images or other data to memory?  Like loading from NOR or
> NAND flash, from USB, IDE or SATA storage devices, from any of the
> supported file system types?

They are not. I just copied the text from loadaddr, but any command
that uses load_addr is missing any upper bounds checking.

> I feel we should either support this consequently everywhere, or not
> at all.  My personal preference is the second option, as otherwise you
> will just add a lot of code in too many places for too little benefit.

I believe the former will make things more robust and I would like to
see this for all commands that load something to memory, but we should
agree on the approach first.

Rob

>
> Best regards,
>
> Wolfgang Denk
>
> --
> DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
> HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
> Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
> No man knows what true happiness is until he gets married.  By  then,
> of course, its too late.