diff mbox series

[v2,1/7] spl: fit: Don't overwrite previous loadable if "load" is missing

Message ID 20210329170516.1893519-2-mr.nuke.me@gmail.com
State Accepted
Commit f0a6ec3656e5561a3d2e246b0fad7e55e419515b
Delegated to: Tom Rini
Headers show
Series SPL: FIT: Bring the SPL_LOAD_FIT path in line with documentation | expand

Commit Message

Alex G. March 29, 2021, 5:05 p.m. UTC 
spl_load_fit_image() will try to load an image at the address given
in the "load" property. Absent such property, it uses

	image_info->load_addr

Correct use of this is demonstrated in spl_fit_append_fdt(), which
resets the 'load_addr' before each spl_load_fit_image() call.

On the other hand loading "loadables" loop in spl_load_simple_fit()
completely ignores this. It re-uses the same structure, but doesn't
reset load_addr. If loadable [i] does not have a "load" property, its
load address defaults to load_addr, which still contains the address
of loadable [i - 1].

A simple solution is to treat NULL as an invalid load address. The
caller can set load_addr = 0 to request an abort if the "load"
property is absent.

Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
Reviewed-by: Simon Glass <sjg@chromium.org>
---
 common/spl/spl_fit.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

Comments

Tom Rini April 16, 2021, 12:25 p.m. UTC | #1 
On Mon, Mar 29, 2021 at 12:05:10PM -0500, Alexandru Gagniuc wrote:

> spl_load_fit_image() will try to load an image at the address given
> in the "load" property. Absent such property, it uses
> 
> 	image_info->load_addr
> 
> Correct use of this is demonstrated in spl_fit_append_fdt(), which
> resets the 'load_addr' before each spl_load_fit_image() call.
> 
> On the other hand loading "loadables" loop in spl_load_simple_fit()
> completely ignores this. It re-uses the same structure, but doesn't
> reset load_addr. If loadable [i] does not have a "load" property, its
> load address defaults to load_addr, which still contains the address
> of loadable [i - 1].
> 
> A simple solution is to treat NULL as an invalid load address. The
> caller can set load_addr = 0 to request an abort if the "load"
> property is absent.
> 
> Signed-off-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> Reviewed-by: Simon Glass <sjg@chromium.org>

Applied to u-boot/master, thanks!
diff mbox series

Patch

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 75c8ff065b..57bcd2b9f1 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -223,7 +223,7 @@  static int get_aligned_image_size(struct spl_load_info *info, int data_size,
  * @image_info:	will be filled with information about the loaded image
  *		If the FIT node does not contain a "load" (address) property,
  *		the image gets loaded to the address pointed to by the
- *		load_addr member in this struct.
+ *		load_addr member in this struct, if load_addr is not 0
  *
  * Return:	0 on success or a negative error number.
  */
@@ -258,8 +258,14 @@  static int spl_load_fit_image(struct spl_load_info *info, ulong sector,
 		debug("%s ", genimg_get_comp_name(image_comp));
 	}
 
-	if (fit_image_get_load(fit, node, &load_addr))
+	if (fit_image_get_load(fit, node, &load_addr)) {
+		if (!image_info->load_addr) {
+			printf("Can't load %s: No load address and no buffer\n",
+			       fit_get_name(fit, node, NULL));
+			return -ENOBUFS;
+		}
 		load_addr = image_info->load_addr;
+	}
 
 	if (!fit_image_get_data_position(fit, node, &offset)) {
 		external_data = true;
@@ -697,6 +703,7 @@  int spl_load_simple_fit(struct spl_image_info *spl_image,
 		if (firmware_node == node)
 			continue;
 
+		image_info.load_addr = 0;
 		ret = spl_load_fit_image(info, sector, &ctx, node, &image_info);
 		if (ret < 0) {
 			printf("%s: can't load image loadables index %d (ret = %d)\n",