[PATCH 3/4] common: bootm: add checks to verify if ramdisk / fdtimage overlaps OS image
Jaehoon Chung
jh80.chung at samsung.com
Tue Oct 20 14:19:54 CEST 2020
On 10/20/20 8:47 PM, Tero Kristo wrote:
> On 20/10/2020 14:07, Jaehoon Chung wrote:
>> 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
>
> I believe you are actually right. However, both checks for rd_end should take the last byte into account, and would need to change the last check to be images.rd_end < start + size in addition to your changes above. Also, we would need to add a check to see if rd_start < start && rd_end >= start + size in addition to everything we check here to cover every possible overlap scenario...
diff --git a/common/bootm.c b/common/bootm.c
index b3377490b3..36273be1cf 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -256,9 +256,11 @@ int bootm_find_images(int flag, int argc, char *const argv[], ulong start,
/* 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))) {
+ (ulong)images.rd_start < start + size) ||
+ ((ulong)images.rd_end > start &&
+ (ulong)images.rd_end <= start + size)) ||
+ ((ulong)images.rd_start < 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 you're ok, then i will send patch for fixing.
Best Regards,
Jaehoon Chung
>
> -Tero
>
>>
>> If i misunderstood something, let me know, plz.
>>
>> Best Regards,
>> Jaehoon Chung
>>
>>>
>>> Signed-off-by: Tero Kristo <t-kristo at 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,
>>>
>>
>
> --
> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
>
More information about the U-Boot
mailing list