On Sun, Oct 11, 2026 at 07:55:41PM -0600, Sebastian Alba Vives wrote:
> spl_fit_images_get_entry() reads the entry point of a /fit-images node
> with fit_image_get_entry(), falling back to fit_image_get_load(). Both
> report failure through their return value and leave the caller's ulong
> untouched: fit_image_get_address() returns -1 before it reaches the
> *load assignment, both when the property is absent and when the value
> does not fit in a target ulong.
>
> The result was stored in ret but never tested, so a node carrying
> neither an entry nor a load property made the function return an
> uninitialised stack value, after first reading it in the debug() call.
> spl_invoke_atf() hands that value to bl31_entry() as the BL33 entry
> point, overwriting the CONFIG_TEXT_BASE default the caller set up for
> exactly this case.
>
> Before commit caa7fc2c5775 ("spl: Use standard FIT entries") the lookup
> used fdt_getprop_u32(), which returns the value itself and yields
> FDT_ERROR on failure, so the variable was always assigned. Switching to
> the fit_image_get_*() accessors moved the failure signal into the
> return value, where it was left unchecked.
>
> Return the entry through an out parameter and report failure, so both
> call sites keep the default they initialised.
>
> Fixes: caa7fc2c5775 ("spl: Use standard FIT entries")
> Signed-off-by: Sebastian Alba Vives <[email protected]>
> ---
> common/spl/spl_atf.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
>
> diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c
> index cb0604ce03..c058774eba 100644
> --- a/common/spl/spl_atf.c
> +++ b/common/spl/spl_atf.c
> @@ -252,17 +252,21 @@ static int spl_fit_images_find(void *blob, int os)
> return found;
> }
>
> -ulong spl_fit_images_get_entry(void *blob, int node)
> +int spl_fit_images_get_entry(void *blob, int node, ulong *entry)
> {
> - ulong val;
> int ret;
>
> - ret = fit_image_get_entry(blob, node, &val);
> + ret = fit_image_get_entry(blob, node, entry);
> if (ret)
> - ret = fit_image_get_load(blob, node, &val);
> + ret = fit_image_get_load(blob, node, entry);
> + if (ret) {
> + debug("%s: no entry or load address\n", __func__);
> + return ret;
> + }
>
> - debug("%s: entry point 0x%lx\n", __func__, val);
> - return val;
> + debug("%s: entry point 0x%lx\n", __func__, *entry);
> +
> + return 0;
> }
>
> void __noreturn spl_invoke_atf(struct spl_image_info *spl_image)
> @@ -280,7 +284,7 @@ void __noreturn spl_invoke_atf(struct spl_image_info
> *spl_image)
> */
> node = spl_fit_images_find(blob, IH_OS_TEE);
> if (node >= 0)
> - bl32_entry = spl_fit_images_get_entry(blob, node);
> + spl_fit_images_get_entry(blob, node, &bl32_entry);
>
> /*
> * Find the U-Boot binary (in /fit-images) load addreess or
> @@ -291,7 +295,7 @@ void __noreturn spl_invoke_atf(struct spl_image_info
> *spl_image)
>
> node = spl_fit_images_find(blob, IH_OS_U_BOOT);
> if (node >= 0)
> - bl33_entry = spl_fit_images_get_entry(blob, node);
> + spl_fit_images_get_entry(blob, node, &bl33_entry);
>
> /*
> * If ATF_NO_PLATFORM_PARAM is set, we override the platformWe change the return type of spl_fit_images_get_entry and now don't check what it returns. We do add a new and useful debug statement. -- Tom
signature.asc
Description: PGP signature
