[PATCH 1/3] bootm: fix uImage crashes when no partition is given

[PATCH 1/3] bootm: fix uImage crashes when no partition is given

[Lucas Stach]

From: Lucas Stach <l.stach@pengutronix.de>

Before the conversion to use the partition string instead of a plain
integer the default was partition 0, now it's just a NULL pointer.

There are a lot of places where strtoul is called on the partition string
which crashes when encountering the NULL pointer. Instead of fixing all
those places, plug in a default partition string that matches the old
behavior by picking partition 0.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 common/bootm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/common/bootm.c b/common/bootm.c
index 7d00f8e..1f99b61 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -465,6 +465,8 @@ static void bootm_print_info(struct image_data *data)
 		printf("OS image not yet relocated\n");
 }
 
+static char part_zero[] = "0";
+
 static int bootm_image_name_and_part(const char *name, char **filename, char **part)
 {
 	char *at, *ret;
@@ -475,7 +477,7 @@ static int bootm_image_name_and_part(const char *name, char **filename, char **p
 	ret = xstrdup(name);
 
 	*filename = ret;
-	*part = NULL;
+	*part = part_zero;
 
 	at = strchr(ret, '@');
 	if (!at)
-- 
2.5.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 2/3] arm: bootm: unify kernel load address calculation

[Lucas Stach]

From: Lucas Stach <l.stach@pengutronix.de>

Instead of having the same logic for uImage and zImage types duplicated
in the code, split it out into a separate function. This does not change
the behavior of the calculation.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm/lib/bootm.c | 74 ++++++++++++++++++++++++++--------------------------
 1 file changed, 37 insertions(+), 37 deletions(-)

diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index 1913d5f..a18d149 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -67,6 +67,34 @@ static int sdram_start_and_size(unsigned long *start, unsigned long *size)
 	return 0;
 }
 
+static void get_kernel_addresses(unsigned long mem_start, size_t image_size,
+				 int verbose, unsigned long *load_address,
+				 unsigned long *spacing)
+{
+	/*
+	 * We don't know the exact decompressed size so just use a conservative
+	 * default of 4 times the size of the compressed image.
+	 */
+	size_t image_decomp_size = PAGE_ALIGN(image_size * 4);
+
+	/*
+	 * By default put oftree/initrd close behind compressed kernel image to
+	 * avoid placing it outside of the kernels lowmem region.
+	 */
+	*spacing = SZ_1M;
+
+	if (*load_address == UIMAGE_INVALID_ADDRESS) {
+		/*
+		 * Place the kernel at an address where it does not need to
+		 * relocate itself before decompression.
+		 */
+		*load_address = mem_start + image_decomp_size;
+		if (verbose)
+			printf("no OS load address, defaulting to 0x%08lx\n",
+				*load_address);
+	}
+}
+
 static int __do_bootm_linux(struct image_data *data, unsigned long free_mem, int swap)
 {
 	unsigned long kernel;
@@ -124,7 +152,7 @@ static int __do_bootm_linux(struct image_data *data, unsigned long free_mem, int
 
 static int do_bootm_linux(struct image_data *data)
 {
-	unsigned long load_address, mem_start, mem_size, mem_free;
+	unsigned long load_address, mem_start, mem_size, mem_free, spacing;
 	int ret;
 
 	ret = sdram_start_and_size(&mem_start, &mem_size);
@@ -133,28 +161,14 @@ static int do_bootm_linux(struct image_data *data)
 
 	load_address = data->os_address;
 
-	if (load_address == UIMAGE_INVALID_ADDRESS) {
-		/*
-		 * Just use a conservative default of 4 times the size of the
-		 * compressed image, to avoid the need for the kernel to
-		 * relocate itself before decompression.
-		 */
-		load_address = mem_start + PAGE_ALIGN(
-		               bootm_get_os_size(data) * 4);
-		if (bootm_verbose(data))
-			printf("no OS load address, defaulting to 0x%08lx\n",
-				load_address);
-	}
+	get_kernel_addresses(mem_start, bootm_get_os_size(data),
+			     bootm_verbose(data), &load_address, &spacing);
 
 	ret = bootm_load_os(data, load_address);
 	if (ret)
 		return ret;
 
-	/*
-	 * put oftree/initrd close behind compressed kernel image to avoid
-	 * placing it outside of the kernels lowmem.
-	 */
-	mem_free = PAGE_ALIGN(data->os_res->end + SZ_1M);
+	mem_free = PAGE_ALIGN(data->os_res->end + spacing);
 
 	return __do_bootm_linux(data, mem_free, 0);
 }
@@ -251,7 +265,7 @@ static int do_bootz_linux(struct image_data *data)
 	u32 end, start;
 	size_t image_size;
 	unsigned long load_address = data->os_address;
-	unsigned long mem_start, mem_size, mem_free;
+	unsigned long mem_start, mem_size, mem_free, spacing;
 
 	ret = sdram_start_and_size(&mem_start, &mem_size);
 	if (ret)
@@ -291,20 +305,10 @@ static int do_bootz_linux(struct image_data *data)
 	}
 
 	image_size = end - start;
+	load_address = data->os_address;
 
-	if (load_address == UIMAGE_INVALID_ADDRESS) {
-		/*
-		 * Just use a conservative default of 4 times the size of the
-		 * compressed image, to avoid the need for the kernel to
-		 * relocate itself before decompression.
-		 */
-		data->os_address = mem_start + PAGE_ALIGN(image_size * 4);
-
-		load_address = data->os_address;
-		if (bootm_verbose(data))
-			printf("no OS load address, defaulting to 0x%08lx\n",
-				load_address);
-	}
+	get_kernel_addresses(mem_start, image_size, bootm_verbose(data),
+			     &load_address, &spacing);
 
 	data->os_res = request_sdram_region("zimage", load_address, image_size);
 	if (!data->os_res) {
@@ -340,11 +344,7 @@ static int do_bootz_linux(struct image_data *data)
 
 	close(fd);
 
-	/*
-	 * put oftree/initrd close behind compressed kernel image to avoid
-	 * placing it outside of the kernels lowmem.
-	 */
-	mem_free = PAGE_ALIGN(data->os_res->end + SZ_1M);
+	mem_free = PAGE_ALIGN(data->os_res->end + spacing);
 
 	return __do_bootm_linux(data, mem_free, swap);
 
-- 
2.5.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

[PATCH 3/3] arm: bootm: be more clever about kernel spacing

[Lucas Stach]

From: Lucas Stach <l.stach@pengutronix.de>

When the kernel load address is chosen by the user/image we need
to check if the kernel needs to relocate itself before decompression.
If that's the case the spacing behind the kernel must allow for this
relocation without overwriting anything placed behind the kernel.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 arch/arm/lib/bootm.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/lib/bootm.c b/arch/arm/lib/bootm.c
index a18d149..28b4f4a 100644
--- a/arch/arm/lib/bootm.c
+++ b/arch/arm/lib/bootm.c
@@ -92,6 +92,14 @@ static void get_kernel_addresses(unsigned long mem_start, size_t image_size,
 		if (verbose)
 			printf("no OS load address, defaulting to 0x%08lx\n",
 				*load_address);
+	} else if (*load_address <= mem_start + image_decomp_size) {
+		/*
+		 * If the user/image specified an address where the kernel needs
+		 * to relocate itself before decompression we need to extend the
+		 * spacing to allow this relocation to happen without
+		 * overwriting anything placed behind the kernel.
+		 */
+		*spacing += image_decomp_size;
 	}
 }
 
-- 
2.5.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] bootm: fix uImage crashes when no partition is given

[Sascha Hauer]

On Fri, Feb 26, 2016 at 08:12:37PM +0100, Lucas Stach wrote:
> From: Lucas Stach <l.stach@pengutronix.de>
> 
> Before the conversion to use the partition string instead of a plain
> integer the default was partition 0, now it's just a NULL pointer.
> 
> There are a lot of places where strtoul is called on the partition string
> which crashes when encountering the NULL pointer. Instead of fixing all
> those places, plug in a default partition string that matches the old
> behavior by picking partition 0.

This was my first approach aswell, but I didn't like the static "0"
string, so I came up with
http://lists.infradead.org/pipermail/barebox/2016-February/026368.html

Sascha

> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  common/bootm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/common/bootm.c b/common/bootm.c
> index 7d00f8e..1f99b61 100644
> --- a/common/bootm.c
> +++ b/common/bootm.c
> @@ -465,6 +465,8 @@ static void bootm_print_info(struct image_data *data)
>  		printf("OS image not yet relocated\n");
>  }
>  
> +static char part_zero[] = "0";
> +
>  static int bootm_image_name_and_part(const char *name, char **filename, char **part)
>  {
>  	char *at, *ret;
> @@ -475,7 +477,7 @@ static int bootm_image_name_and_part(const char *name, char **filename, char **p
>  	ret = xstrdup(name);
>  
>  	*filename = ret;
> -	*part = NULL;
> +	*part = part_zero;
>  
>  	at = strchr(ret, '@');
>  	if (!at)
> -- 
> 2.5.0
> 
> 
> _______________________________________________
> barebox mailing list
> barebox@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/barebox
> 

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] bootm: fix uImage crashes when no partition is given

[Lucas Stach]

Am Montag, den 29.02.2016, 08:48 +0100 schrieb Sascha Hauer:
> On Fri, Feb 26, 2016 at 08:12:37PM +0100, Lucas Stach wrote:
> > From: Lucas Stach <l.stach@pengutronix.de>
> > 
> > Before the conversion to use the partition string instead of a plain
> > integer the default was partition 0, now it's just a NULL pointer.
> > 
> > There are a lot of places where strtoul is called on the partition string
> > which crashes when encountering the NULL pointer. Instead of fixing all
> > those places, plug in a default partition string that matches the old
> > behavior by picking partition 0.
> 
> This was my first approach aswell, but I didn't like the static "0"
> string, so I came up with
> http://lists.infradead.org/pipermail/barebox/2016-February/026368.html
> 
Fix looks good.

Can you drop this one patch, but apply the other 2 from this series?

Regards,
Lucas

-- 
Pengutronix e.K.             | Lucas Stach                 |
Industrial Linux Solutions   | http://www.pengutronix.de/  |


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH 1/3] bootm: fix uImage crashes when no partition is given

[Sascha Hauer]

On Mon, Feb 29, 2016 at 10:05:35AM +0100, Lucas Stach wrote:
> Am Montag, den 29.02.2016, 08:48 +0100 schrieb Sascha Hauer:
> > On Fri, Feb 26, 2016 at 08:12:37PM +0100, Lucas Stach wrote:
> > > From: Lucas Stach <l.stach@pengutronix.de>
> > > 
> > > Before the conversion to use the partition string instead of a plain
> > > integer the default was partition 0, now it's just a NULL pointer.
> > > 
> > > There are a lot of places where strtoul is called on the partition string
> > > which crashes when encountering the NULL pointer. Instead of fixing all
> > > those places, plug in a default partition string that matches the old
> > > behavior by picking partition 0.
> > 
> > This was my first approach aswell, but I didn't like the static "0"
> > string, so I came up with
> > http://lists.infradead.org/pipermail/barebox/2016-February/026368.html
> > 
> Fix looks good.
> 
> Can you drop this one patch, but apply the other 2 from this series?

Just did that. I just wanted to give it a few tests before doing it.

Sascha


-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox