Dear Tero,
On 6/12/20 9:41 PM, Tero Kristo wrote:
> These cases are typically fatal and are difficult to debug for random
> users. Add checks for detecting overlapping images and abort if overlap
> is detected.
I have a question about your patch.. because I have confused...
So i want to clear what i missed.
During booting with ramdisk, my target is stopped.
ramdisk start address = 0x2700000
size = 0x800000
kernel start - 0x2F00000
bootm 0x2f00000 0x2700000:0x800000 0x02600000
ramdisk.end is ramdisk_start + size (0x2F00000)
Then it will be used until 0x2efffff, not 0x2f00000.
When i have checked, it doesn't overwrite RD image.
> + /* check if ramdisk overlaps OS image */
> + if (images.rd_start && (((ulong)images.rd_start >= start &&
> + (ulong)images.rd_start <= start + size) ||
> + ((ulong)images.rd_end >= start &&
> + (ulong)images.rd_end <= start + size))) {
> + printf("ERROR: RD image overlaps OS image (OS=0x%lx..0x%lx)\n",
> + start, start + size);> + return 1;
> + }
Because of hit this condition...So doesn't boot...
I think that it needs to change the below conditions..
images.rd_start < start + size or images.rd_start < start + size - 1.
images.rd_end > start or image.rd_end - 1 >= start
If i misunderstood something, let me know, plz.
Best Regards,
Jaehoon Chung
>
> Signed-off-by: Tero Kristo <t-kri...@ti.com>
> ---
> cmd/booti.c | 2 +-
> cmd/bootz.c | 2 +-
> common/bootm.c | 29 +++++++++++++++++++++++++++--
> common/bootm_os.c | 4 ++--
> include/bootm.h | 3 ++-
> 5 files changed, 33 insertions(+), 7 deletions(-)
>
> diff --git a/cmd/booti.c b/cmd/booti.c
> index ae37975494..6153b229cf 100644
> --- a/cmd/booti.c
> +++ b/cmd/booti.c
> @@ -93,7 +93,7 @@ static int booti_start(struct cmd_tbl *cmdtp, int flag, int
> argc,
> * Handle the BOOTM_STATE_FINDOTHER state ourselves as we do not
> * have a header that provide this informaiton.
> */
> - if (bootm_find_images(flag, argc, argv))
> + if (bootm_find_images(flag, argc, argv, relocated_addr, image_size))
> return 1;
>
> return 0;
> diff --git a/cmd/bootz.c b/cmd/bootz.c
> index bc15fd8ec4..1c8b0cf89f 100644
> --- a/cmd/bootz.c
> +++ b/cmd/bootz.c
> @@ -54,7 +54,7 @@ static int bootz_start(struct cmd_tbl *cmdtp, int flag, int
> argc,
> * Handle the BOOTM_STATE_FINDOTHER state ourselves as we do not
> * have a header that provide this informaiton.
> */
> - if (bootm_find_images(flag, argc, argv))
> + if (bootm_find_images(flag, argc, argv, zi_start, zi_end - zi_start))
> return 1;
>
> return 0;
> diff --git a/common/bootm.c b/common/bootm.c
> index 2ed7521520..247b600d9c 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -228,6 +228,8 @@ static int bootm_find_os(struct cmd_tbl *cmdtp, int flag,
> int argc,
> * @flag: Ignored Argument
> * @argc: command argument count
> * @argv: command argument list
> + * @start: OS image start address
> + * @size: OS image size
> *
> * boot_find_images() will attempt to load an available ramdisk,
> * flattened device tree, as well as specifically marked
> @@ -239,7 +241,8 @@ static int bootm_find_os(struct cmd_tbl *cmdtp, int flag,
> int argc,
> * 0, if all existing images were loaded correctly
> * 1, if an image is found but corrupted, or invalid
> */
> -int bootm_find_images(int flag, int argc, char *const argv[])
> +int bootm_find_images(int flag, int argc, char *const argv[], ulong start,
> + ulong size)
> {
> int ret;
>
> @@ -251,6 +254,16 @@ int bootm_find_images(int flag, int argc, char *const
> argv[])
> return 1;
> }
>
> + /* check if ramdisk overlaps OS image */
> + if (images.rd_start && (((ulong)images.rd_start >= start &&
> + (ulong)images.rd_start <= start + size) ||
> + ((ulong)images.rd_end >= start &&
> + (ulong)images.rd_end <= start + size))) {
> + printf("ERROR: RD image overlaps OS image (OS=0x%lx..0x%lx)\n",
> + start, start + size);
> + return 1;
> + }
> +
> #if IMAGE_ENABLE_OF_LIBFDT
> /* find flattened device tree */
> ret = boot_get_fdt(flag, argc, argv, IH_ARCH_DEFAULT, &images,
> @@ -259,6 +272,18 @@ int bootm_find_images(int flag, int argc, char *const
> argv[])
> puts("Could not find a valid device tree\n");
> return 1;
> }
> +
> + /* check if FDT overlaps OS image */
> + if (images.ft_addr &&
> + (((ulong)images.ft_addr >= start &&
> + (ulong)images.ft_addr <= start + size) ||
> + ((ulong)images.ft_addr + images.ft_len >= start &&
> + (ulong)images.ft_addr + images.ft_len <= start + size))) {
> + printf("ERROR: FDT image overlaps OS image (OS=0x%lx..0x%lx)\n",
> + start, start + size);
> + return 1;
> + }
> +
> if (CONFIG_IS_ENABLED(CMD_FDT))
> set_working_fdt_addr(map_to_sysmem(images.ft_addr));
> #endif
> @@ -294,7 +319,7 @@ static int bootm_find_other(struct cmd_tbl *cmdtp, int
> flag, int argc,
> (images.os.type == IH_TYPE_MULTI)) &&
> (images.os.os == IH_OS_LINUX ||
> images.os.os == IH_OS_VXWORKS))
> - return bootm_find_images(flag, argc, argv);
> + return bootm_find_images(flag, argc, argv, 0, 0);
>
> return 0;
> }
> diff --git a/common/bootm_os.c b/common/bootm_os.c
> index 55296483f7..a3c360e80a 100644
> --- a/common/bootm_os.c
> +++ b/common/bootm_os.c
> @@ -495,7 +495,7 @@ static int do_bootm_tee(int flag, int argc, char *const
> argv[],
> return ret;
>
> /* Locate FDT etc */
> - ret = bootm_find_images(flag, argc, argv);
> + ret = bootm_find_images(flag, argc, argv, 0, 0);
> if (ret)
> return ret;
>
> @@ -516,7 +516,7 @@ static int do_bootm_efi(int flag, int argc, char *const
> argv[],
> return 0;
>
> /* Locate FDT, if provided */
> - ret = bootm_find_images(flag, argc, argv);
> + ret = bootm_find_images(flag, argc, argv, 0, 0);
> if (ret)
> return ret;
>
> diff --git a/include/bootm.h b/include/bootm.h
> index 1e7f29e134..0350c349f3 100644
> --- a/include/bootm.h
> +++ b/include/bootm.h
> @@ -53,7 +53,8 @@ int boot_selected_os(int argc, char *const argv[], int
> state,
> ulong bootm_disable_interrupts(void);
>
> /* This is a special function used by booti/bootz */
> -int bootm_find_images(int flag, int argc, char *const argv[]);
> +int bootm_find_images(int flag, int argc, char *const argv[], ulong start,
> + ulong size);
>
> int do_bootm_states(struct cmd_tbl *cmdtp, int flag, int argc,
> char *const argv[], int states, bootm_headers_t *images,
>
