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 platform
-- 
2.43.0

Reply via email to