[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