[PATCH v2 1/8] qcom: board: validate fdt before trying to use it

[Caleb Connolly]

On 06/03/2024 00:52, Volodymyr Babchuk wrote:
> There are cases when previous bootloader stage leaves some seemingly
> valid value in r0, which in fact does not point to valid FDT
> blob. This behavior was encountered when trying to boot U-Boot as
> "hyp" loader on SA8155P-ADP.
> 
> To be sure that we really got the pointer to a device tree we need to
> validate it with fdt_valid() function.

Thanks for this!
> 
> Note: This approach is not 100% fool-proof, as get_prev_bl_fdt_addr()
> theoretically can return a pointer to a region that is not physically
> mapped and we will get data abort exception when "fdt_valid" will try
> to access it. But at this early boot stage we don't know where RAM is
> anyways so there is little we can do.

IME this hasn't been an issue in practise, but yes it is a risk.
> 
> Signed-off-by: Volodymyr Babchuk <volodymyr_babchuk at epam.com>
> 
> ---
> 
> Changes in v2:
>  - New patch in v2
> 
>  arch/arm/mach-snapdragon/board.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm/mach-snapdragon/board.c b/arch/arm/mach-snapdragon/board.c
> index f12f5791a1..10eec47ada 100644
> --- a/arch/arm/mach-snapdragon/board.c
> +++ b/arch/arm/mach-snapdragon/board.c
> @@ -24,6 +24,7 @@
>  #include <linux/sizes.h>
>  #include <lmb.h>
>  #include <malloc.h>
> +#include <fdt_support.h>
>  #include <usb.h>
>  #include <sort.h>
>  
> @@ -93,7 +94,9 @@ void *board_fdt_blob_setup(int *err)
>  	 * try and use the FDT built into U-Boot if there is one...
>  	 * This avoids having a hard dependency on the previous stage bootloader
>  	 */
> -	if (IS_ENABLED(CONFIG_OF_SEPARATE) && (!fdt || fdt != ALIGN(fdt, SZ_4K))) {
> +
> +	if (IS_ENABLED(CONFIG_OF_SEPARATE) && (!fdt || fdt != ALIGN(fdt, SZ_4K) ||
> +					       !fdt_valid((void *)&fdt))) {
Please use fdt_check_header() here, fdt_valid() prints a bunch of stuff
using printf() which isn't really useful here.
>  		debug("%s: Using built in FDT, bootloader gave us %#llx\n", __func__, fdt);
>  		return (void *)gd->fdt_blob;
>  	}

-- 
// Caleb (they/them)