All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/4] Some reserved memory hardenings
@ 2020-06-12 12:41 Tero Kristo
  2020-06-12 12:41 ` [PATCH 1/4] lmb/bdinfo: dump lmb info via bdinfo Tero Kristo
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Tero Kristo @ 2020-06-12 12:41 UTC (permalink / raw)
  To: u-boot

Hi,

This series applies some reserved memory checks to the boot commands,
and enhances the bdinfo command to dump out reserved memory sections.
Motivation behind these is mostly to help detecting overwriting either
DTB, ramdisk or Linux image with other images. Right now, the boot in
most cases just continues but will hang before anything is printed out
from Linux. In some cases the boot continues even if u-boot detects
something that is terribly wrong (like DTB has been overwritten.)

Patch #3 has some holes in it (it does not pass the OS image / size in
some cases.) I wasn't quite sure what parameter to pass in these cases
so I just left them at zero, meaning no checks are done.

The layout of the reserved memory section printout with patch #1 could
maybe also made look bit neater, I just re-used the existing lmb debug
functionality for this purpose.

-Tero


--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] lmb/bdinfo: dump lmb info via bdinfo
  2020-06-12 12:41 [PATCH 0/4] Some reserved memory hardenings Tero Kristo
@ 2020-06-12 12:41 ` Tero Kristo
  2020-07-17 13:29   ` Tom Rini
  2020-06-12 12:41 ` [PATCH 2/4] image: fdt: bail out with error if no boot time FDT image found Tero Kristo
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Tero Kristo @ 2020-06-12 12:41 UTC (permalink / raw)
  To: u-boot

Dump lmb status from the bdinfo command. This is useful for seeing the
reserved memory regions from the u-boot cmdline.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 cmd/bdinfo.c  |  8 +++++++-
 include/lmb.h |  1 +
 lib/lmb.c     | 42 +++++++++++++++++++++++-------------------
 3 files changed, 31 insertions(+), 20 deletions(-)

diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
index dba552b03f..05e1b27de0 100644
--- a/cmd/bdinfo.c
+++ b/cmd/bdinfo.c
@@ -10,6 +10,7 @@
 #include <common.h>
 #include <command.h>
 #include <env.h>
+#include <lmb.h>
 #include <net.h>
 #include <vsprintf.h>
 #include <asm/cache.h>
@@ -365,8 +366,13 @@ static int do_bdinfo(struct cmd_tbl *cmdtp, int flag, int argc,
 #if CONFIG_IS_ENABLED(MULTI_DTB_FIT)
 	print_num("multi_dtb_fit", (ulong)gd->multi_dtb_fit);
 #endif
-	if (gd->fdt_blob)
+	if (gd->fdt_blob) {
+		struct lmb lmb;
 		print_num("fdt_blob", (ulong)gd->fdt_blob);
+		lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
+		lmb_dump_all_force(&lmb);
+	}
+
 	print_cpu_word_size();
 
 	return 0;
diff --git a/include/lmb.h b/include/lmb.h
index 3b338dfee0..0d90ab1d65 100644
--- a/include/lmb.h
+++ b/include/lmb.h
@@ -48,6 +48,7 @@ extern int lmb_is_reserved(struct lmb *lmb, phys_addr_t addr);
 extern long lmb_free(struct lmb *lmb, phys_addr_t base, phys_size_t size);
 
 extern void lmb_dump_all(struct lmb *lmb);
+extern void lmb_dump_all_force(struct lmb *lmb);
 
 static inline phys_size_t
 lmb_size_bytes(struct lmb_region *type, unsigned long region_nr)
diff --git a/lib/lmb.c b/lib/lmb.c
index 008bcc7930..d62f25f9de 100644
--- a/lib/lmb.c
+++ b/lib/lmb.c
@@ -14,33 +14,37 @@
 
 #define LMB_ALLOC_ANYWHERE	0
 
-void lmb_dump_all(struct lmb *lmb)
+void lmb_dump_all_force(struct lmb *lmb)
 {
-#ifdef DEBUG
 	unsigned long i;
 
-	debug("lmb_dump_all:\n");
-	debug("    memory.cnt		   = 0x%lx\n", lmb->memory.cnt);
-	debug("    memory.size		   = 0x%llx\n",
-	      (unsigned long long)lmb->memory.size);
+	printf("lmb_dump_all:\n");
+	printf("    memory.cnt		   = 0x%lx\n", lmb->memory.cnt);
+	printf("    memory.size		   = 0x%llx\n",
+	       (unsigned long long)lmb->memory.size);
 	for (i = 0; i < lmb->memory.cnt; i++) {
-		debug("    memory.reg[0x%lx].base   = 0x%llx\n", i,
-		      (unsigned long long)lmb->memory.region[i].base);
-		debug("		   .size   = 0x%llx\n",
-		      (unsigned long long)lmb->memory.region[i].size);
+		printf("    memory.reg[0x%lx].base   = 0x%llx\n", i,
+		       (unsigned long long)lmb->memory.region[i].base);
+		printf("		   .size   = 0x%llx\n",
+		       (unsigned long long)lmb->memory.region[i].size);
 	}
 
-	debug("\n    reserved.cnt	   = 0x%lx\n",
-		lmb->reserved.cnt);
-	debug("    reserved.size	   = 0x%llx\n",
-		(unsigned long long)lmb->reserved.size);
+	printf("\n    reserved.cnt	   = 0x%lx\n", lmb->reserved.cnt);
+	printf("    reserved.size	   = 0x%llx\n",
+	       (unsigned long long)lmb->reserved.size);
 	for (i = 0; i < lmb->reserved.cnt; i++) {
-		debug("    reserved.reg[0x%lx].base = 0x%llx\n", i,
-		      (unsigned long long)lmb->reserved.region[i].base);
-		debug("		     .size = 0x%llx\n",
-		      (unsigned long long)lmb->reserved.region[i].size);
+		printf("    reserved.reg[0x%lx].base = 0x%llx\n", i,
+		       (unsigned long long)lmb->reserved.region[i].base);
+		printf("		     .size = 0x%llx\n",
+		       (unsigned long long)lmb->reserved.region[i].size);
 	}
-#endif /* DEBUG */
+}
+
+void lmb_dump_all(struct lmb *lmb)
+{
+#ifdef DEBUG
+	lmb_dump_all_force(lmb);
+#endif
 }
 
 static long lmb_addrs_overlap(phys_addr_t base1, phys_size_t size1,
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 2/4] image: fdt: bail out with error if no boot time FDT image found
  2020-06-12 12:41 [PATCH 0/4] Some reserved memory hardenings Tero Kristo
  2020-06-12 12:41 ` [PATCH 1/4] lmb/bdinfo: dump lmb info via bdinfo Tero Kristo
@ 2020-06-12 12:41 ` Tero Kristo
  2020-07-17 20:57   ` Tom Rini
  2020-06-12 12:41 ` [PATCH 3/4] common: bootm: add checks to verify if ramdisk / fdtimage overlaps OS image Tero Kristo
  2020-06-12 12:41 ` [PATCH 4/4] cmd: booti: convert the debug print about image move to printf Tero Kristo
  3 siblings, 1 reply; 13+ messages in thread
From: Tero Kristo @ 2020-06-12 12:41 UTC (permalink / raw)
  To: u-boot

Currently the boot continues if the FDT image is clearly corrupted,
which just causes the loaded OS to hang. Abort boot properly if the FDT
is corrupted.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 common/image-fdt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/common/image-fdt.c b/common/image-fdt.c
index b63e772bd6..7005b34966 100644
--- a/common/image-fdt.c
+++ b/common/image-fdt.c
@@ -426,7 +426,7 @@ int boot_get_fdt(int flag, int argc, char *const argv[], uint8_t arch,
 			break;
 		default:
 			puts("ERROR: Did not find a cmdline Flattened Device Tree\n");
-			goto no_fdt;
+			goto error;
 		}
 
 		printf("   Booting using the fdt blob at %#08lx\n", fdt_addr);
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 3/4] common: bootm: add checks to verify if ramdisk / fdtimage overlaps OS image
  2020-06-12 12:41 [PATCH 0/4] Some reserved memory hardenings Tero Kristo
  2020-06-12 12:41 ` [PATCH 1/4] lmb/bdinfo: dump lmb info via bdinfo Tero Kristo
  2020-06-12 12:41 ` [PATCH 2/4] image: fdt: bail out with error if no boot time FDT image found Tero Kristo
@ 2020-06-12 12:41 ` Tero Kristo
  2020-07-17 20:57   ` Tom Rini
  2020-10-20 11:07   ` Jaehoon Chung
  2020-06-12 12:41 ` [PATCH 4/4] cmd: booti: convert the debug print about image move to printf Tero Kristo
  3 siblings, 2 replies; 13+ messages in thread
From: Tero Kristo @ 2020-06-12 12:41 UTC (permalink / raw)
  To: u-boot

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.

Signed-off-by: Tero Kristo <t-kristo@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,
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 4/4] cmd: booti: convert the debug print about image move to printf
  2020-06-12 12:41 [PATCH 0/4] Some reserved memory hardenings Tero Kristo
                   ` (2 preceding siblings ...)
  2020-06-12 12:41 ` [PATCH 3/4] common: bootm: add checks to verify if ramdisk / fdtimage overlaps OS image Tero Kristo
@ 2020-06-12 12:41 ` Tero Kristo
  2020-07-17 20:57   ` Tom Rini
  3 siblings, 1 reply; 13+ messages in thread
From: Tero Kristo @ 2020-06-12 12:41 UTC (permalink / raw)
  To: u-boot

Moving of the OS image may have some nasty side effects like corrupting
DTB. Convert the current debug print to printf so that the relocation of
the OS is always obvious to the user.

Signed-off-by: Tero Kristo <t-kristo@ti.com>
---
 cmd/booti.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cmd/booti.c b/cmd/booti.c
index 6153b229cf..ad04ebd8c3 100644
--- a/cmd/booti.c
+++ b/cmd/booti.c
@@ -79,7 +79,8 @@ static int booti_start(struct cmd_tbl *cmdtp, int flag, int argc,
 
 	/* Handle BOOTM_STATE_LOADOS */
 	if (relocated_addr != ld) {
-		debug("Moving Image from 0x%lx to 0x%lx\n", ld, relocated_addr);
+		printf("Moving Image from 0x%lx to 0x%lx, end=%lx\n", ld,
+		       relocated_addr, relocated_addr + image_size);
 		memmove((void *)relocated_addr, (void *)ld, image_size);
 	}
 
-- 
2.17.1

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH 1/4] lmb/bdinfo: dump lmb info via bdinfo
  2020-06-12 12:41 ` [PATCH 1/4] lmb/bdinfo: dump lmb info via bdinfo Tero Kristo
@ 2020-07-17 13:29   ` Tom Rini
  2020-07-17 13:45     ` Tero Kristo
  0 siblings, 1 reply; 13+ messages in thread
From: Tom Rini @ 2020-07-17 13:29 UTC (permalink / raw)
  To: u-boot

On Fri, Jun 12, 2020 at 03:41:18PM +0300, Tero Kristo wrote:

> Dump lmb status from the bdinfo command. This is useful for seeing the
> reserved memory regions from the u-boot cmdline.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>
> ---
>  cmd/bdinfo.c  |  8 +++++++-
>  include/lmb.h |  1 +
>  lib/lmb.c     | 42 +++++++++++++++++++++++-------------------
>  3 files changed, 31 insertions(+), 20 deletions(-)
> 
> diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
> index dba552b03f..05e1b27de0 100644
> --- a/cmd/bdinfo.c
> +++ b/cmd/bdinfo.c
> @@ -10,6 +10,7 @@
>  #include <common.h>
>  #include <command.h>
>  #include <env.h>
> +#include <lmb.h>
>  #include <net.h>
>  #include <vsprintf.h>
>  #include <asm/cache.h>
> @@ -365,8 +366,13 @@ static int do_bdinfo(struct cmd_tbl *cmdtp, int flag, int argc,
>  #if CONFIG_IS_ENABLED(MULTI_DTB_FIT)
>  	print_num("multi_dtb_fit", (ulong)gd->multi_dtb_fit);
>  #endif
> -	if (gd->fdt_blob)
> +	if (gd->fdt_blob) {
> +		struct lmb lmb;
>  		print_num("fdt_blob", (ulong)gd->fdt_blob);
> +		lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
> +		lmb_dump_all_force(&lmb);
> +	}
> +
>  	print_cpu_word_size();
>  
>  	return 0;

Can you please rebase this on top of master?  Simon's series to cleanup
bdinfo stuff has been applied, thanks.  The rest of your changes are
fine and under test now.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200717/0fa9e377/attachment.sig>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 1/4] lmb/bdinfo: dump lmb info via bdinfo
  2020-07-17 13:29   ` Tom Rini
@ 2020-07-17 13:45     ` Tero Kristo
  0 siblings, 0 replies; 13+ messages in thread
From: Tero Kristo @ 2020-07-17 13:45 UTC (permalink / raw)
  To: u-boot

On 17/07/2020 16:29, Tom Rini wrote:
> On Fri, Jun 12, 2020 at 03:41:18PM +0300, Tero Kristo wrote:
> 
>> Dump lmb status from the bdinfo command. This is useful for seeing the
>> reserved memory regions from the u-boot cmdline.
>>
>> Signed-off-by: Tero Kristo <t-kristo@ti.com>
>> ---
>>   cmd/bdinfo.c  |  8 +++++++-
>>   include/lmb.h |  1 +
>>   lib/lmb.c     | 42 +++++++++++++++++++++++-------------------
>>   3 files changed, 31 insertions(+), 20 deletions(-)
>>
>> diff --git a/cmd/bdinfo.c b/cmd/bdinfo.c
>> index dba552b03f..05e1b27de0 100644
>> --- a/cmd/bdinfo.c
>> +++ b/cmd/bdinfo.c
>> @@ -10,6 +10,7 @@
>>   #include <common.h>
>>   #include <command.h>
>>   #include <env.h>
>> +#include <lmb.h>
>>   #include <net.h>
>>   #include <vsprintf.h>
>>   #include <asm/cache.h>
>> @@ -365,8 +366,13 @@ static int do_bdinfo(struct cmd_tbl *cmdtp, int flag, int argc,
>>   #if CONFIG_IS_ENABLED(MULTI_DTB_FIT)
>>   	print_num("multi_dtb_fit", (ulong)gd->multi_dtb_fit);
>>   #endif
>> -	if (gd->fdt_blob)
>> +	if (gd->fdt_blob) {
>> +		struct lmb lmb;
>>   		print_num("fdt_blob", (ulong)gd->fdt_blob);
>> +		lmb_init_and_reserve(&lmb, gd->bd, (void *)gd->fdt_blob);
>> +		lmb_dump_all_force(&lmb);
>> +	}
>> +
>>   	print_cpu_word_size();
>>   
>>   	return 0;
> 
> Can you please rebase this on top of master?  Simon's series to cleanup
> bdinfo stuff has been applied, thanks.  The rest of your changes are
> fine and under test now.

Yeah, I can repost this single patch. Thanks for the heads-up.

-Tero

--
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 2/4] image: fdt: bail out with error if no boot time FDT image found
  2020-06-12 12:41 ` [PATCH 2/4] image: fdt: bail out with error if no boot time FDT image found Tero Kristo
@ 2020-07-17 20:57   ` Tom Rini
  0 siblings, 0 replies; 13+ messages in thread
From: Tom Rini @ 2020-07-17 20:57 UTC (permalink / raw)
  To: u-boot

On Fri, Jun 12, 2020 at 03:41:19PM +0300, Tero Kristo wrote:

> Currently the boot continues if the FDT image is clearly corrupted,
> which just causes the loaded OS to hang. Abort boot properly if the FDT
> is corrupted.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200717/0a2c2d76/attachment.sig>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/4] common: bootm: add checks to verify if ramdisk / fdtimage overlaps OS image
  2020-06-12 12:41 ` [PATCH 3/4] common: bootm: add checks to verify if ramdisk / fdtimage overlaps OS image Tero Kristo
@ 2020-07-17 20:57   ` Tom Rini
  2020-10-20 11:07   ` Jaehoon Chung
  1 sibling, 0 replies; 13+ messages in thread
From: Tom Rini @ 2020-07-17 20:57 UTC (permalink / raw)
  To: u-boot

On Fri, Jun 12, 2020 at 03:41:20PM +0300, 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.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200717/bd911fa3/attachment.sig>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 4/4] cmd: booti: convert the debug print about image move to printf
  2020-06-12 12:41 ` [PATCH 4/4] cmd: booti: convert the debug print about image move to printf Tero Kristo
@ 2020-07-17 20:57   ` Tom Rini
  0 siblings, 0 replies; 13+ messages in thread
From: Tom Rini @ 2020-07-17 20:57 UTC (permalink / raw)
  To: u-boot

On Fri, Jun 12, 2020 at 03:41:21PM +0300, Tero Kristo wrote:

> Moving of the OS image may have some nasty side effects like corrupting
> DTB. Convert the current debug print to printf so that the relocation of
> the OS is always obvious to the user.
> 
> Signed-off-by: Tero Kristo <t-kristo@ti.com>

Applied to u-boot/master, thanks!

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200717/b174f49d/attachment.sig>

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/4] common: bootm: add checks to verify if ramdisk / fdtimage overlaps OS image
  2020-06-12 12:41 ` [PATCH 3/4] common: bootm: add checks to verify if ramdisk / fdtimage overlaps OS image Tero Kristo
  2020-07-17 20:57   ` Tom Rini
@ 2020-10-20 11:07   ` Jaehoon Chung
  2020-10-20 11:47     ` Tero Kristo
  1 sibling, 1 reply; 13+ messages in thread
From: Jaehoon Chung @ 2020-10-20 11:07 UTC (permalink / raw)
  To: u-boot

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-kristo@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,
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/4] common: bootm: add checks to verify if ramdisk / fdtimage overlaps OS image
  2020-10-20 11:07   ` Jaehoon Chung
@ 2020-10-20 11:47     ` Tero Kristo
  2020-10-20 12:19       ` Jaehoon Chung
  0 siblings, 1 reply; 13+ messages in thread
From: Tero Kristo @ 2020-10-20 11:47 UTC (permalink / raw)
  To: u-boot

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...

-Tero

> 
> If i misunderstood something, let me know, plz.
> 
> Best Regards,
> Jaehoon Chung
> 
>>
>> Signed-off-by: Tero Kristo <t-kristo@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

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH 3/4] common: bootm: add checks to verify if ramdisk / fdtimage overlaps OS image
  2020-10-20 11:47     ` Tero Kristo
@ 2020-10-20 12:19       ` Jaehoon Chung
  0 siblings, 0 replies; 13+ messages in thread
From: Jaehoon Chung @ 2020-10-20 12:19 UTC (permalink / raw)
  To: u-boot

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@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
> 

^ permalink raw reply related	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-10-20 12:19 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-12 12:41 [PATCH 0/4] Some reserved memory hardenings Tero Kristo
2020-06-12 12:41 ` [PATCH 1/4] lmb/bdinfo: dump lmb info via bdinfo Tero Kristo
2020-07-17 13:29   ` Tom Rini
2020-07-17 13:45     ` Tero Kristo
2020-06-12 12:41 ` [PATCH 2/4] image: fdt: bail out with error if no boot time FDT image found Tero Kristo
2020-07-17 20:57   ` Tom Rini
2020-06-12 12:41 ` [PATCH 3/4] common: bootm: add checks to verify if ramdisk / fdtimage overlaps OS image Tero Kristo
2020-07-17 20:57   ` Tom Rini
2020-10-20 11:07   ` Jaehoon Chung
2020-10-20 11:47     ` Tero Kristo
2020-10-20 12:19       ` Jaehoon Chung
2020-06-12 12:41 ` [PATCH 4/4] cmd: booti: convert the debug print about image move to printf Tero Kristo
2020-07-17 20:57   ` Tom Rini

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.