diff mbox series

[U-Boot,v8,10/10] arm: bootm: fix sp detection at end of address range

Message ID 20181217200830.32585-11-simon.k.r.goldschmidt@gmail.com
State Superseded
Headers show
Series Fix CVE-2018-18440 and CVE-2018-18439 | expand

Commit Message

Simon Goldschmidt Dec. 17, 2018, 8:08 p.m. UTC
This fixes  'arch_lmb_reserve()' for ARM that tries to detect in which
DRAM bank 'sp' is in.

This code failed if a bank was at the end of physical address range
(i.e. size + length overflowed to 0).

To fix this, calculate 'bank_end' as 'size + length - 1' so that such
banks end at 0xffffffff, not 0.

Fixes: 15751403b6 ("ARM: bootm: don't assume sp is in DRAM bank 0")
Reported-by: Frank Wunderlich <frank-w@public-files.de>
Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
---

Changes in v8:
- this patch is new in v8

Changes in v7: None
Changes in v6: None
Changes in v5: None
Changes in v4: None
Changes in v2: None

 arch/arm/lib/bootm.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Stephen Warren Dec. 17, 2018, 8:58 p.m. UTC | #1
On 12/17/18 1:08 PM, Simon Goldschmidt wrote:
> This fixes  'arch_lmb_reserve()' for ARM that tries to detect in which
> DRAM bank 'sp' is in.
> 
> This code failed if a bank was at the end of physical address range
> (i.e. size + length overflowed to 0).
> 
> To fix this, calculate 'bank_end' as 'size + length - 1' so that such
> banks end at 0xffffffff, not 0.
> 
> Fixes: 15751403b6 ("ARM: bootm: don't assume sp is in DRAM bank 0")
> Reported-by: Frank Wunderlich <frank-w@public-files.de>
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>

This patch,
Reviewed-by: Stephen Warren <swarren@nvidia.com>
Frank Wunderlich Dec. 18, 2018, 3:55 p.m. UTC | #2
Hi,

seems that Patch0 does not receive Mailinglist and my answer to it got blocked...

i get:

arch/arm/lib/bootm.c: In function ‘arch_lmb_reserve’:
arch/arm/lib/bootm.c:67:8: error: ‘bi_dram’ undeclared (first use in this function); did you mean ‘blk_dread’?
   if (!bi_dram[bank].size || sp < gd->bd->bi_dram[bank].start)
        ^~~~~~~
        blk_dread

which i fixed this way:

diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index e9a333af0e..48bb41249e 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -64,7 +64,7 @@ void arch_lmb_reserve(struct lmb *lmb)
        /* adjust sp by 4K to be safe */
        sp -= 4096;
        for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
-               if (!bi_dram[bank].size || sp < gd->bd->bi_dram[bank].start)
+               if (!gd->bd->bi_dram[bank].size || sp < gd->bd->bi_dram[bank].start)
                        continue;
                /* Watch out for RAM at end of address space! */
                bank_end = gd->bd->bi_dram[bank].start +

after that at least sp_start is protected

U-Boot> bd
arch_number = 0x00000000
boot_params = 0x80000100
DRAM bank   = 0x00000000
-> start    = 0x80000000
-> size     = 0x80000000
current eth = unknown
ip_addr     = <NULL>
baudrate    = 115200 bps
TLB addr    = 0xFFFF0000
relocaddr   = 0xFFF9C000
reloc off   = 0x7E19C000
irq_sp      = 0xFFB9AED0
sp start    = 0xFFB9AEC0
Early malloc usage: 318 / 4000
fdt_blob = fffd0be0

U-Boot> setenv scriptaddr 0xFFB9AEC0
U-Boot> fatload ${device} ${partition} ${scriptaddr} ${bpi}/${board}/${service}/${bootenv};
** Reading file would overwrite reserved memory **

same with relocaddr -x

U-Boot> setenv scriptaddr 0xFFF9BF00
U-Boot> fatload ${device} ${partition} ${scriptaddr} ${bpi}/${board}/${service}/
${bootenv};
** Reading file would overwrite reserved memory **

regards Frank

> Gesendet: Montag, 17. Dezember 2018 um 21:08 Uhr
> Von: "Simon Goldschmidt" <simon.k.r.goldschmidt@gmail.com>
> An: "Tom Rini" <trini@konsulko.com>, u-boot@lists.denx.de, "Joe Hershberger" <joe.hershberger@ni.com>
> Cc: "Heinrich Schuchardt" <xypron.glpk@gmx.de>, "Fabio Estevam" <festevam@gmail.com>, "Andrea Barisani" <andrea.barisani@f-secure.com>, "Frank Wunderlich" <frank-w@public-files.de>, "Simon Goldschmidt" <simon.k.r.goldschmidt@gmail.com>, "Simon Glass" <sjg@chromium.org>, "Masahiro Yamada" <yamada.masahiro@socionext.com>, "Tom Warren" <twarren@nvidia.com>, "Bin Meng" <bmeng.cn@gmail.com>, "Vasyl Vavrychuk" <vvavrychuk@gmail.com>, "Albert Aribaud" <albert.u.boot@aribaud.net>, "Hannes Schmelzer" <hannes.schmelzer@br-automation.com>, "Stephen Warren" <swarren@nvidia.com>
> Betreff: [PATCH v8 10/10] arm: bootm: fix sp detection at end of address range
>
> This fixes  'arch_lmb_reserve()' for ARM that tries to detect in which
> DRAM bank 'sp' is in.
> 
> This code failed if a bank was at the end of physical address range
> (i.e. size + length overflowed to 0).
> 
> To fix this, calculate 'bank_end' as 'size + length - 1' so that such
> banks end at 0xffffffff, not 0.
> 
> Fixes: 15751403b6 ("ARM: bootm: don't assume sp is in DRAM bank 0")
> Reported-by: Frank Wunderlich <frank-w@public-files.de>
> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
> ---
> 
> Changes in v8:
> - this patch is new in v8
> 
> Changes in v7: None
> Changes in v6: None
> Changes in v5: None
> Changes in v4: None
> Changes in v2: None
> 
>  arch/arm/lib/bootm.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index c3c1d2fdfa..e9a333af0e 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -64,13 +64,14 @@ void arch_lmb_reserve(struct lmb *lmb)
>  	/* adjust sp by 4K to be safe */
>  	sp -= 4096;
>  	for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
> -		if (sp < gd->bd->bi_dram[bank].start)
> +		if (!bi_dram[bank].size || sp < gd->bd->bi_dram[bank].start)
>  			continue;
> +		/* Watch out for RAM at end of address space! */
>  		bank_end = gd->bd->bi_dram[bank].start +
> -			gd->bd->bi_dram[bank].size;
> -		if (sp >= bank_end)
> +			gd->bd->bi_dram[bank].size - 1;
> +		if (sp > bank_end)
>  			continue;
> -		lmb_reserve(lmb, sp, bank_end - sp);
> +		lmb_reserve(lmb, sp, bank_end - sp + 1);
>  		break;
>  	}
>  }
> -- 
> 2.17.1
> 
>
Simon Goldschmidt Dec. 18, 2018, 7:50 p.m. UTC | #3
Am 18.12.2018 um 16:55 schrieb Frank Wunderlich:
> Hi,
> 
> seems that Patch0 does not receive Mailinglist and my answer to it got blocked...
> 
> i get:
> 
> arch/arm/lib/bootm.c: In function ‘arch_lmb_reserve’:
> arch/arm/lib/bootm.c:67:8: error: ‘bi_dram’ undeclared (first use in this function); did you mean ‘blk_dread’?
>     if (!bi_dram[bank].size || sp < gd->bd->bi_dram[bank].start)
>          ^~~~~~~
>          blk_dread
> 
> which i fixed this way:
> 
> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
> index e9a333af0e..48bb41249e 100644
> --- a/arch/arm/lib/bootm.c
> +++ b/arch/arm/lib/bootm.c
> @@ -64,7 +64,7 @@ void arch_lmb_reserve(struct lmb *lmb)
>          /* adjust sp by 4K to be safe */
>          sp -= 4096;
>          for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
> -               if (!bi_dram[bank].size || sp < gd->bd->bi_dram[bank].start)
> +               if (!gd->bd->bi_dram[bank].size || sp < gd->bd->bi_dram[bank].start)
>                          continue;
>                  /* Watch out for RAM at end of address space! */
>                  bank_end = gd->bd->bi_dram[bank].start +
> 

D'Oh! Don't know how that could have slipped through. Seems I was a 
little hasty in adding this last patch :-(

Your fix is of course correct. I'll submit v9 :-/

> after that at least sp_start is protected
> 
> U-Boot> bd
> arch_number = 0x00000000
> boot_params = 0x80000100
> DRAM bank   = 0x00000000
> -> start    = 0x80000000
> -> size     = 0x80000000
> current eth = unknown
> ip_addr     = <NULL>
> baudrate    = 115200 bps
> TLB addr    = 0xFFFF0000
> relocaddr   = 0xFFF9C000
> reloc off   = 0x7E19C000
> irq_sp      = 0xFFB9AED0
> sp start    = 0xFFB9AEC0
> Early malloc usage: 318 / 4000
> fdt_blob = fffd0be0
> 
> U-Boot> setenv scriptaddr 0xFFB9AEC0
> U-Boot> fatload ${device} ${partition} ${scriptaddr} ${bpi}/${board}/${service}/${bootenv};
> ** Reading file would overwrite reserved memory **
> 
> same with relocaddr -x
> 
> U-Boot> setenv scriptaddr 0xFFF9BF00
> U-Boot> fatload ${device} ${partition} ${scriptaddr} ${bpi}/${board}/${service}/
> ${bootenv};
> ** Reading file would overwrite reserved memory **

So, yay, it finally works for you!

Thanks for testing!

Regards,
Simon

> 
> regards Frank
> 
>> Gesendet: Montag, 17. Dezember 2018 um 21:08 Uhr
>> Von: "Simon Goldschmidt" <simon.k.r.goldschmidt@gmail.com>
>> An: "Tom Rini" <trini@konsulko.com>, u-boot@lists.denx.de, "Joe Hershberger" <joe.hershberger@ni.com>
>> Cc: "Heinrich Schuchardt" <xypron.glpk@gmx.de>, "Fabio Estevam" <festevam@gmail.com>, "Andrea Barisani" <andrea.barisani@f-secure.com>, "Frank Wunderlich" <frank-w@public-files.de>, "Simon Goldschmidt" <simon.k.r.goldschmidt@gmail.com>, "Simon Glass" <sjg@chromium.org>, "Masahiro Yamada" <yamada.masahiro@socionext.com>, "Tom Warren" <twarren@nvidia.com>, "Bin Meng" <bmeng.cn@gmail.com>, "Vasyl Vavrychuk" <vvavrychuk@gmail.com>, "Albert Aribaud" <albert.u.boot@aribaud.net>, "Hannes Schmelzer" <hannes.schmelzer@br-automation.com>, "Stephen Warren" <swarren@nvidia.com>
>> Betreff: [PATCH v8 10/10] arm: bootm: fix sp detection at end of address range
>>
>> This fixes  'arch_lmb_reserve()' for ARM that tries to detect in which
>> DRAM bank 'sp' is in.
>>
>> This code failed if a bank was at the end of physical address range
>> (i.e. size + length overflowed to 0).
>>
>> To fix this, calculate 'bank_end' as 'size + length - 1' so that such
>> banks end at 0xffffffff, not 0.
>>
>> Fixes: 15751403b6 ("ARM: bootm: don't assume sp is in DRAM bank 0")
>> Reported-by: Frank Wunderlich <frank-w@public-files.de>
>> Signed-off-by: Simon Goldschmidt <simon.k.r.goldschmidt@gmail.com>
>> ---
>>
>> Changes in v8:
>> - this patch is new in v8
>>
>> Changes in v7: None
>> Changes in v6: None
>> Changes in v5: None
>> Changes in v4: None
>> Changes in v2: None
>>
>>   arch/arm/lib/bootm.c | 9 +++++----
>>   1 file changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
>> index c3c1d2fdfa..e9a333af0e 100644
>> --- a/arch/arm/lib/bootm.c
>> +++ b/arch/arm/lib/bootm.c
>> @@ -64,13 +64,14 @@ void arch_lmb_reserve(struct lmb *lmb)
>>   	/* adjust sp by 4K to be safe */
>>   	sp -= 4096;
>>   	for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
>> -		if (sp < gd->bd->bi_dram[bank].start)
>> +		if (!bi_dram[bank].size || sp < gd->bd->bi_dram[bank].start)
>>   			continue;
>> +		/* Watch out for RAM at end of address space! */
>>   		bank_end = gd->bd->bi_dram[bank].start +
>> -			gd->bd->bi_dram[bank].size;
>> -		if (sp >= bank_end)
>> +			gd->bd->bi_dram[bank].size - 1;
>> +		if (sp > bank_end)
>>   			continue;
>> -		lmb_reserve(lmb, sp, bank_end - sp);
>> +		lmb_reserve(lmb, sp, bank_end - sp + 1);
>>   		break;
>>   	}
>>   }
>> -- 
>> 2.17.1
>>
>>
diff mbox series

Patch

diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index c3c1d2fdfa..e9a333af0e 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -64,13 +64,14 @@  void arch_lmb_reserve(struct lmb *lmb)
 	/* adjust sp by 4K to be safe */
 	sp -= 4096;
 	for (bank = 0; bank < CONFIG_NR_DRAM_BANKS; bank++) {
-		if (sp < gd->bd->bi_dram[bank].start)
+		if (!bi_dram[bank].size || sp < gd->bd->bi_dram[bank].start)
 			continue;
+		/* Watch out for RAM at end of address space! */
 		bank_end = gd->bd->bi_dram[bank].start +
-			gd->bd->bi_dram[bank].size;
-		if (sp >= bank_end)
+			gd->bd->bi_dram[bank].size - 1;
+		if (sp > bank_end)
 			continue;
-		lmb_reserve(lmb, sp, bank_end - sp);
+		lmb_reserve(lmb, sp, bank_end - sp + 1);
 		break;
 	}
 }