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

[Wolfgang Denk]

Dear Rob Herring,

In message <CAL_JsqJ3mDsUoqqDL93Aa6OiLVpscVs3M_ixjgF0O+yfkftgEA at mail.gmail.com> you wrote:
>
> > 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.

This can always happen - like when the user sets the maximum load size
too big.  Actually the concept of a maximum load size is not working
at all, as it takes not into account _where_ you load the image to;
when loading it low in RAM the allowable size is different from when
loading to a higher address.  You cannot tell in advance how much RAM
you can actually use - the stack is growing downward, and we don't
have any hard limits on the stack size.  And even if the loading does
not collide with the stack, you don't know if the stack will not grow
urther down when you process the image, in which case it would get
overwritten _after_ loading, so your test would not catch it either.

> fastboot is loading images to boot just like other commands that use
> loadaddr, but has the additional need to know the max size. Rather

Don't you see that the max actually available size depends on
loadaddr, so you cannot set one without considering the other?

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

Yes, and this is intentionally.  There is no clean, reliable way to
implement this features, so instead of providing some deceptive
security we rely on the user knowing what he is doing.

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

Well, we need one that actually works first.

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
Let the programmers be many and the managers few -- then all will  be
productive.               -- Geoffrey James, "The Tao of Programming"