From mboxrd@z Thu Jan 1 00:00:00 1970 From: Simon Glass Date: Mon, 15 Mar 2021 20:25:17 +1300 Subject: [PATCH 1/7] spl: fit: Don't overwrite previous loadable if "load" is missing In-Reply-To: <20210310180442.746517-2-mr.nuke.me@gmail.com> References: <20210310180442.746517-1-mr.nuke.me@gmail.com> <20210310180442.746517-2-mr.nuke.me@gmail.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Alexandru, On Wed, 10 Mar 2021 at 11:04, 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. Another way would be to add a load_valid bool or flag to the struct. > > Signed-off-by: Alexandru Gagniuc > --- > common/spl/spl_fit.c | 11 +++++++++-- > 1 file changed, 9 insertions(+), 2 deletions(-) > > diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c > index 75c8ff065b..43fc43cf2b 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 NULL s/NULL/0/ This is not a pointer, but an address. > * > * 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", > -- > 2.26.2 > Reviewed-by: Simon Glass Regards, Simon