[SLOF] [PATCH v2 02/11] obp-tftp: Make sure to not overwrite paflof in memory

[Greg Kurz]

On Wed, 23 May 2018 08:03:13 +0200
Thomas Huth <thuth at redhat.com> wrote:

> On 22.05.2018 17:01, Greg Kurz wrote:
> > On Fri, 18 May 2018 17:45:31 +0200
> > Thomas Huth <thuth at redhat.com> wrote:
> >   
> >> The obp-tftp package is currently using an arbitrary large value
> >> as maximal load size. If the downloaded file is big enough, we
> >> can easily erase Paflof in memory this way. Let's make sure that
> >> this can not happen by limiting the size to the amount of memory
> >> below the Paflof binary (which is close to the end of the RAM)
> >> in case of board-qemu, or the amount of memory between the minimum
> >> RAM size and the load-base on board-js2x.
> >>
> >> Signed-off-by: Thomas Huth <thuth at redhat.com>
> >> ---
> >>  slof/fs/packages/obp-tftp.fs | 8 +++++++-
> >>  1 file changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/slof/fs/packages/obp-tftp.fs b/slof/fs/packages/obp-tftp.fs
> >> index 19c11e1..7236624 100644
> >> --- a/slof/fs/packages/obp-tftp.fs
> >> +++ b/slof/fs/packages/obp-tftp.fs
> >> @@ -24,7 +24,13 @@ s" obp-tftp" device-name
> >>      my-parent ihandle>phandle node>path encode-string
> >>      s" bootpath" set-chosen
> >>  
> >> -    60000000                        ( addr maxlen )
> >> +    \ Determine the maximum size that we can load:
> >> +    dup paflof-start < IF
> >> +        paflof-start
> >> +    ELSE
> >> +        MIN-RAM-SIZE  
> > 
> > Hmm... IIUC this is a size, not an address...  
> 
> Depending on how you look at it, it's both, a size and an address. It's
> the size of a RAM block starting at address 0. So the address of the
> first byte after this ram block is 0 + MIN-RAM-SIZE = MIN-RAM-SIZE.
> 

Ok, I had missed the true meaning of MIN-RAM-SIZE obviously :)

> >> +    THEN                                  ( addr endaddr )
> >> +    over -                                ( addr maxlen )
> >>    
> > 
> > ... so we should rather have this, shouldn't we ?
> > 
> >     \ Determine the maximum size that we can load:
> >     dup paflof-start < IF
> >         paflof-start
> >         over -
> >     ELSE
> >         MIN-RAM-SIZE
> >     THEN                                  ( addr maxlen )  
> 
> No, you still got to consider the load-base (the "addr" in above stack
> comments) here:
> 
>     +-------------+-------------------------------------+
>     ^             ^                                     ^
>     |             |                                     |
>  Address 0    load-base                            MIN-RAM-SIZE
>                ( addr )
> 
> The amount of data that we can load via TFTP is the size between the
> load-base and MIN-RAM-SIZE, not the size between address 0 and MIN-RAM-SIZE.
> 

And so your computation is right of course.

Reviewed-by: Greg Kurz <groug at kaod.org>

>  Thomas
>