diff mbox

[U-Boot,01/23] image: Use ram_top, not bi_memsize, in getenv_bootm_size

Message ID 20160926182917.27531-2-paul.burton@imgtec.com
State Superseded
Delegated to: Daniel Schwierzeck
Headers show

Commit Message

Paul Burton Sept. 26, 2016, 6:28 p.m. UTC
When determining the region of memory to allow for use by bootm, using
bi_memstart & adding bi_memsize can cause problems if that leads to an
integer overflow. For example on some MIPS systems bi_memstart would be
0xffffffff80000000 (ie. the start of the MIPS ckseg0 region) and if the
system has 2GB of memory then the addition would wrap around to 0.

The maximum amount of memory to be used by U-Boot is already accounted
for by the ram_top field of struct global_data, so make use of that for
the calculation instead.

Signed-off-by: Paul Burton <paul.burton@imgtec.com>
---

 common/image.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Daniel Schwierzeck Sept. 27, 2016, 11:03 a.m. UTC | #1
+cc Simon and Tom

2016-09-26 20:28 GMT+02:00 Paul Burton <paul.burton@imgtec.com>:
> When determining the region of memory to allow for use by bootm, using
> bi_memstart & adding bi_memsize can cause problems if that leads to an
> integer overflow. For example on some MIPS systems bi_memstart would be
> 0xffffffff80000000 (ie. the start of the MIPS ckseg0 region) and if the
> system has 2GB of memory then the addition would wrap around to 0.
>
> The maximum amount of memory to be used by U-Boot is already accounted
> for by the ram_top field of struct global_data, so make use of that for
> the calculation instead.
>
> Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> ---
>
>  common/image.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/common/image.c b/common/image.c
> index a5d19ab..30537cd 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -489,7 +489,7 @@ phys_size_t getenv_bootm_size(void)
>         size = gd->bd->bi_dram[0].size;
>  #else
>         start = gd->bd->bi_memstart;
> -       size = gd->bd->bi_memsize;
> +       size = gd->ram_top - start;
>  #endif
>
>         s = getenv("bootm_low");
> --
> 2.10.0
>

I think the code in the #if branch above should updated too to keep a
consistent behaviour
Tom Rini Sept. 27, 2016, 2:44 p.m. UTC | #2
On Tue, Sep 27, 2016 at 01:03:47PM +0200, Daniel Schwierzeck wrote:
> +cc Simon and Tom
> 
> 2016-09-26 20:28 GMT+02:00 Paul Burton <paul.burton@imgtec.com>:
> > When determining the region of memory to allow for use by bootm, using
> > bi_memstart & adding bi_memsize can cause problems if that leads to an
> > integer overflow. For example on some MIPS systems bi_memstart would be
> > 0xffffffff80000000 (ie. the start of the MIPS ckseg0 region) and if the
> > system has 2GB of memory then the addition would wrap around to 0.
> >
> > The maximum amount of memory to be used by U-Boot is already accounted
> > for by the ram_top field of struct global_data, so make use of that for
> > the calculation instead.
> >
> > Signed-off-by: Paul Burton <paul.burton@imgtec.com>
> > ---
> >
> >  common/image.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/common/image.c b/common/image.c
> > index a5d19ab..30537cd 100644
> > --- a/common/image.c
> > +++ b/common/image.c
> > @@ -489,7 +489,7 @@ phys_size_t getenv_bootm_size(void)
> >         size = gd->bd->bi_dram[0].size;
> >  #else
> >         start = gd->bd->bi_memstart;
> > -       size = gd->bd->bi_memsize;
> > +       size = gd->ram_top - start;
> >  #endif
> >
> >         s = getenv("bootm_low");
> 
> I think the code in the #if branch above should updated too to keep a
> consistent behaviour

The top half of the else is for ARM, and the bottom is everyone else.
So this needs testing and acks outside of MIPS.  But yes, I think we
should use size = gd->ram_top - start for everyone.
diff mbox

Patch

diff --git a/common/image.c b/common/image.c
index a5d19ab..30537cd 100644
--- a/common/image.c
+++ b/common/image.c
@@ -489,7 +489,7 @@  phys_size_t getenv_bootm_size(void)
 	size = gd->bd->bi_dram[0].size;
 #else
 	start = gd->bd->bi_memstart;
-	size = gd->bd->bi_memsize;
+	size = gd->ram_top - start;
 #endif
 
 	s = getenv("bootm_low");