Hi Richard,

On Fri, 12 Jul 2024 at 09:11, Richard Weinberger richard@nod.at wrote:

bootstage_get_size() returns the total size of the data structure including associated records. When copying from gd->bootstage, only the allocation size of gd->bootstage must be used. Otherwise too much memory is copied.

This bug caused no harm so far because gd->new_bootstage is always large enough and reading beyond the allocation length of gd->bootstage caused no problem due to the U-Boot memory layout.

Fix by using the correct size and perform the initial copy directly in bootstage_relocate() to have the whole relocation process in the same function.

Nice commit message.

Can you use 'bootstage' as the commit tag?

Signed-off-by: Richard Weinberger richard@nod.at

common/board_f.c | 6 ------ common/bootstage.c | 7 ++++++- 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/common/board_f.c b/common/board_f.c index 039d6d712d..f4d87692b9 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -683,12 +683,6 @@ static int reloc_bootstage(void) if (gd->flags & GD_FLG_SKIP_RELOC) return 0; if (gd->new_bootstage) {

•           int size = bootstage_get_size();
  

•           debug("Copying bootstage from %p to %p, size %x\n",
  

•                 gd->bootstage, gd->new_bootstage, size);
  

•           memcpy(gd->new_bootstage, gd->bootstage, size);
  

•           gd->bootstage = gd->new_bootstage;
            bootstage_relocate();
    }
  

#endif diff --git a/common/bootstage.c b/common/bootstage.c index 0e6d80718f..aea5a318df 100644 --- a/common/bootstage.c +++ b/common/bootstage.c @@ -58,10 +58,15 @@ struct bootstage_hdr {

int bootstage_relocate(void) {

•   struct bootstage_data *data = gd->bootstage;
  

•   struct bootstage_data *data;
    int i;
    char *ptr;
  
  

•   debug("Copying bootstage from %p to %p\n", gd->bootstage,
  

•         gd->new_bootstage);
  

•   memcpy(gd->new_bootstage, gd->bootstage, sizeof(struct bootstage_data));
  

I would like to have the relocation addresses in board_f like with other relocations, so it is easy to see what is happening, in one file. So how about passing the old address to bootstage_relocate() so it doesn't need to access gd->new_bootstage ?

•   data = gd->bootstage = gd->new_bootstage;
  

•   /* Figure out where to relocate the strings to */
    ptr = (char *)(data + 1);
  
  

-- 2.35.3

Regards, Simon