mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH master] ARM: stm32mp: much enlarge very early stack size
@ 2023-02-17 17:51 Ahmad Fatoum
  2023-02-21 10:16 ` Sascha Hauer
  0 siblings, 1 reply; 2+ messages in thread
From: Ahmad Fatoum @ 2023-02-17 17:51 UTC (permalink / raw)
  To: barebox; +Cc: Ahmad Fatoum

The barebox stack is always located at a fixed offset from the end of
SDRAM. To determine end of SDRAM, barebox may need a stack setup, so
at that early time, either stack pointer set up by BootROM is reused
or we set up a temporary stack on some on-chip SRAM .

On STM32MP1, TF-A runs after BootROM, so barebox runs in normal world
without prepared stack and TF-A is located on on-chip SRAM.

So what we did so far was have a small stack of 64 bytes starting behind
end of barebox image to cover the first function call to determine
actual memory size. This may have been chosen that small with the thought
that an overflow would give an error message anyway, because of failed
decompression. An overflow of exactly 4 bytes overwrites just the barebox
proper image size though located at the end of the barebox image, thereby
overflowing the size and leading to out-of-bounds access during decompression
in 1 of 4 cases depending on barebox size, because we align barebox
proper size to 16 bytes...

Let's just use a fully sized stack even in the early state. That's
usually 32K and we expect a barebox loaded into DRAM to always have
that much bytes following it. Another way would've been to use
some of the other SRAMs, but I decided to leave those alone, as not
to clobber data used by the coprocessor, when reloading barebox.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
 arch/arm/mach-stm32mp/include/mach/entry.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mach-stm32mp/include/mach/entry.h b/arch/arm/mach-stm32mp/include/mach/entry.h
index f8d981cca7cd..8d3adb4bdaf9 100644
--- a/arch/arm/mach-stm32mp/include/mach/entry.h
+++ b/arch/arm/mach-stm32mp/include/mach/entry.h
@@ -11,7 +11,7 @@ static __always_inline void stm32mp_cpu_lowlevel_init(void)
 	unsigned long stack_top;
 	arm_cpu_lowlevel_init();
 
-	stack_top = (unsigned long)__image_end + get_runtime_offset() + 64;
+	stack_top = (unsigned long)__image_end + get_runtime_offset() + CONFIG_STACK_SIZE;
 	stack_top = ALIGN(stack_top, 16);
 	arm_setup_stack(stack_top);
 }
-- 
2.30.2




^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH master] ARM: stm32mp: much enlarge very early stack size
  2023-02-17 17:51 [PATCH master] ARM: stm32mp: much enlarge very early stack size Ahmad Fatoum
@ 2023-02-21 10:16 ` Sascha Hauer
  0 siblings, 0 replies; 2+ messages in thread
From: Sascha Hauer @ 2023-02-21 10:16 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: barebox

On Fri, Feb 17, 2023 at 06:51:24PM +0100, Ahmad Fatoum wrote:
> The barebox stack is always located at a fixed offset from the end of
> SDRAM. To determine end of SDRAM, barebox may need a stack setup, so
> at that early time, either stack pointer set up by BootROM is reused
> or we set up a temporary stack on some on-chip SRAM .
> 
> On STM32MP1, TF-A runs after BootROM, so barebox runs in normal world
> without prepared stack and TF-A is located on on-chip SRAM.
> 
> So what we did so far was have a small stack of 64 bytes starting behind
> end of barebox image to cover the first function call to determine
> actual memory size. This may have been chosen that small with the thought
> that an overflow would give an error message anyway, because of failed
> decompression. An overflow of exactly 4 bytes overwrites just the barebox
> proper image size though located at the end of the barebox image, thereby
> overflowing the size and leading to out-of-bounds access during decompression
> in 1 of 4 cases depending on barebox size, because we align barebox
> proper size to 16 bytes...
> 
> Let's just use a fully sized stack even in the early state. That's
> usually 32K and we expect a barebox loaded into DRAM to always have
> that much bytes following it. Another way would've been to use
> some of the other SRAMs, but I decided to leave those alone, as not
> to clobber data used by the coprocessor, when reloading barebox.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
>  arch/arm/mach-stm32mp/include/mach/entry.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Applied, thanks

Sascha

> 
> diff --git a/arch/arm/mach-stm32mp/include/mach/entry.h b/arch/arm/mach-stm32mp/include/mach/entry.h
> index f8d981cca7cd..8d3adb4bdaf9 100644
> --- a/arch/arm/mach-stm32mp/include/mach/entry.h
> +++ b/arch/arm/mach-stm32mp/include/mach/entry.h
> @@ -11,7 +11,7 @@ static __always_inline void stm32mp_cpu_lowlevel_init(void)
>  	unsigned long stack_top;
>  	arm_cpu_lowlevel_init();
>  
> -	stack_top = (unsigned long)__image_end + get_runtime_offset() + 64;
> +	stack_top = (unsigned long)__image_end + get_runtime_offset() + CONFIG_STACK_SIZE;
>  	stack_top = ALIGN(stack_top, 16);
>  	arm_setup_stack(stack_top);
>  }
> -- 
> 2.30.2
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2023-02-21 10:18 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-17 17:51 [PATCH master] ARM: stm32mp: much enlarge very early stack size Ahmad Fatoum
2023-02-21 10:16 ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox