[U-Boot] [PATCH] Makefile: ensure DTB doesn't overflow into initial stack

[Simon Glass]

Hi Stephen,

On 9 January 2018 at 11:52, Stephen Warren <swarren at wwwdotorg.org> wrote:
>
> From: Stephen Warren <swarren at nvidia.com>
>
> With CONFIG_SYS_INIT_SP_BSS_OFFSET enabled, the initial (pre-relocation)
> stack is placed some distance after bss_start. The control DTB is appended
> to the U-Boot binary at bss_start. If the DTB is too large, or the SP BSS
> offset too small, then the initial stack could corrupt the DTB. Enhance
> the Makefile to check whether this is likely to occur.
>
> Signed-off-by: Stephen Warren <swarren at nvidia.com>
> ---
> This builds on top of my previous patch "ARMv8: Allow dynamic early stack
> pointer". However, since all the logic is conditional and only activated
> if CONFIG_SYS_INIT_SP_BSS_OFFSET is defined, it can be applied with or
> without that other patch. It'd make sense to apply it afterwards and in
> the same branch though, or the change won't make a lot of sense to someone
> reading history in order.
>
>  Makefile | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/Makefile b/Makefile
> index d8f419bcd900..52cd6ea72161 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -811,6 +811,10 @@ ifneq ($(CONFIG_BUILD_TARGET),)
>  ALL-y += $(CONFIG_BUILD_TARGET:"%"=%)
>  endif
>
> +ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),)
> +ALL-y += init_sp_bss_offset_check
> +endif
> +
>  LDFLAGS_u-boot += $(LDFLAGS_FINAL)
>
>  # Avoid 'Not enough room for program headers' error on binutils 2.28 onwards.
> @@ -939,6 +943,33 @@ binary_size_check: u-boot-nodtb.bin FORCE
>                 fi \
>         fi
>
> +ifneq ($(CONFIG_SYS_INIT_SP_BSS_OFFSET),)
> +ifneq ($(CONFIG_SYS_MALLOC_F_LEN),)
> +subtract_sys_malloc_f_len = space=$$(($${space} - $(CONFIG_SYS_MALLOC_F_LEN)))
> +else
> +subtract_sys_malloc_f_len = true
> +endif
> +# The 1/4 margin below is somewhat arbitrary. The likely initial SP usage is
> +# so low that the DTB could probably use 90%+ of the available space, for
> +# current values of CONFIG_SYS_INIT_SP_BSS_OFFSET at least. However, let's be
> +# safe for now and tweak this later if space becomes tight.
> +# A rejected alternative would be to check that some absolute minimum stack
> +# space was available. However, since CONFIG_SYS_INIT_SP_BSS_OFFSET is
> +# deliberately build-specific, to take account of build-to-build stack usage
> +# differences due to different feature sets, there is no common absolute value
> +# to check against.
> +init_sp_bss_offset_check: u-boot.dtb FORCE
> +       @dtb_size=$(shell wc -c u-boot.dtb | awk '{print $$1}') ; \
> +       space=$(CONFIG_SYS_INIT_SP_BSS_OFFSET) ; \
> +       $(subtract_sys_malloc_f_len) ; \
> +       quarter_space=$$(($${space} / 4)) ; \
> +       if [ $${dtb_size} -gt $${quarter_space} ]; then \
> +               echo "u-boot.dtb is larger than 1 quarter of " >&2 ; \
> +               echo "(CONFIG_SYS_INIT_SP_BSS_OFFSET - CONFIG_SYS_MALLOC_F_LEN)" >&2 ; \
> +               exit 1 ; \
> +       fi
> +endif
> +
>  u-boot-nodtb.bin: u-boot FORCE
>         $(call if_changed,objcopy)
>         $(call DO_STATIC_RELA,$<,$@,$(CONFIG_SYS_TEXT_BASE))
> --
> 2.15.1
>

Looking at the code here I wonder if it would be easy to check this in binman?

Regards,
Simon