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.

Pass the entry point out through a parameter instead. The accessors
leave it untouched when they fail, so both call sites keep the default
they initialised, and the uninitialised read is gone. A failed lookup
is not an error the caller can act on - the default is the fallback -
so the function returns void rather than a status nobody would check.

Fixes: caa7fc2c5775 ("spl: Use standard FIT entries")
Signed-off-by: Sebastian Alba Vives <[email protected]>
---
 common/spl/spl_atf.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/common/spl/spl_atf.c b/common/spl/spl_atf.c
index cb0604ce03..c1233b54cf 100644
--- a/common/spl/spl_atf.c
+++ b/common/spl/spl_atf.c
@@ -252,17 +252,20 @@ static int spl_fit_images_find(void *blob, int os)
        return found;
 }
 
-ulong spl_fit_images_get_entry(void *blob, int node)
+void 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);
-
-       debug("%s: entry point 0x%lx\n", __func__, val);
-       return val;
+               ret = fit_image_get_load(blob, node, entry);
+       if (ret) {
+               debug("%s: no entry or load address, keeping 0x%lx\n",
+                     __func__, *entry);
+               return;
+       }
+
+       debug("%s: entry point 0x%lx\n", __func__, *entry);
 }
 
 void __noreturn spl_invoke_atf(struct spl_image_info *spl_image)
@@ -280,7 +283,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 +294,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