bootstage: Fix out-of-bounds read in reloc_bootstage()

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.

Signed-off-by: Richard Weinberger <richard@nod.at>
Reviewed-by: Simon Glass <sjg@chromium.org>

diff --git a/common/board_f.c b/common/board_f.c
index d71005d9f835219e28bcd6001d63b75138fe3874..454426d921c7049689328f9ccd6be71cdd0d20b2 100644 (file)
--- a/common/board_f.c
+++ b/common/board_f.c
@@ -684,13 +684,7 @@ 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();
+               bootstage_relocate(gd->new_bootstage);
        }
 #endif
 

diff --git a/common/bootstage.c b/common/bootstage.c
index b6c268d9f4782ec2dd283ea22230c7786f9df987..49acc9078a6d3280784c2cb562a72aa0fe56ce65 100644 (file)
--- a/common/bootstage.c
+++ b/common/bootstage.c
@@ -54,12 +54,16 @@ struct bootstage_hdr {
        u32 next_id;            /* Next ID to use for bootstage */
 };
 
-int bootstage_relocate(void)
+int bootstage_relocate(void *to)
 {
-       struct bootstage_data *data = gd->bootstage;
+       struct bootstage_data *data;
        int i;
        char *ptr;
 
+       debug("Copying bootstage from %p to %p\n", gd->bootstage, to);
+       memcpy(to, gd->bootstage, sizeof(struct bootstage_data));
+       data = gd->bootstage = to;
+
        /* Figure out where to relocate the strings to */
        ptr = (char *)(data + 1);
 

diff --git a/include/bootstage.h b/include/bootstage.h
index f4e77b09d747cf733838a9b7df47c363f1c9fdcc..57792648c4909e1bf8c791f9d95ab4187aff58e9 100644 (file)
--- a/include/bootstage.h
+++ b/include/bootstage.h
@@ -258,7 +258,7 @@ void show_boot_progress(int val);
  * relocation, since memory can be overwritten later.
  * Return: Always returns 0, to indicate success
  */
-int bootstage_relocate(void);
+int bootstage_relocate(void *to);
 
 /**
  * Add a new bootstage record
@@ -395,7 +395,7 @@ static inline ulong bootstage_add_record(enum bootstage_id id,
  * and won't even do that unless CONFIG_SHOW_BOOT_PROGRESS is defined
  */
 
-static inline int bootstage_relocate(void)
+static inline int bootstage_relocate(void *to)
 {
        return 0;
 }