[PATCH] kvx: exclude dtb from malloc zone

[PATCH] kvx: exclude dtb from malloc zone

[Clement Leger]

When dtb is provided by the FSBL (first stage bootloader), it might
end up in the malloc zone initialized by barebox. While the dtb is
discarded after being unflatten, if the dtb is provided right after
barebox elf, then the malloc can overwrite it while unflattening it when
allocating nodes.
To avoid that, exclude the dtb from the malloc zone by checking if it
overlaps it. If so, determine the largest zone for the allocation and
modify the memory area to use that.

Signed-off-by: Clement Leger <cleger@kalray.eu>
---
 arch/kvx/lib/board.c | 43 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 41 insertions(+), 2 deletions(-)

diff --git a/arch/kvx/lib/board.c b/arch/kvx/lib/board.c
index 4d6ca6983..78fa83e02 100644
--- a/arch/kvx/lib/board.c
+++ b/arch/kvx/lib/board.c
@@ -103,15 +103,54 @@ err:
 	while (1);
 }
 
+/**
+ * exclude_dtb_from_alloc - Find best zone to allocate without overwriting dtb
+ *
+ * @fdt: fdt blob
+ * @alloc_start: start of allocation zone
+ * @alloc_end: end of allocation zone
+ */
+static void exclude_dtb_from_alloc(void *fdt, u64 *alloc_start, u64 *alloc_end)
+{
+	const struct fdt_header *fdth = fdt;
+	u64 fdt_start = (u64) fdt;
+	u64 fdt_end = fdt_start + be32_to_cpu(fdth->totalsize);
+	u64 start_size = 0, end_size = 0;
+
+	/*
+	 * If the device tree is inside the malloc zone, we must exclude it to
+	 * avoid allocating memory over it while unflattening it
+	 */
+	if (fdt_end < *alloc_start || fdt_start > (*alloc_end))
+		return;
+
+	/* Compute the largest remaining chunk when removing the dtb */
+	if (fdt_start >= *alloc_start)
+		start_size = (fdt_start - *alloc_start);
+
+	if (fdt_end <= *alloc_end)
+		end_size = *alloc_end - fdt_end;
+
+	/* Modify start/end to reflect the maximum area we found */
+	if (start_size >= end_size)
+		*alloc_end = fdt_start;
+	else
+		*alloc_start = fdt_end;
+}
+
 void __noreturn kvx_start_barebox(void)
 {
 	u64 memsize = 0, membase = 0;
 	u64 barebox_text_end = (u64) &__end;
+	u64 alloc_start, alloc_end;
 
 	of_find_mem(boot_dtb, barebox_text_end, &membase, &memsize);
 
-	mem_malloc_init((void *) barebox_text_end,
-			(void *) (membase + memsize));
+	alloc_start = barebox_text_end;
+	alloc_end = (membase + memsize);
+	exclude_dtb_from_alloc(boot_dtb, &alloc_start, &alloc_end);
+
+	mem_malloc_init((void *) alloc_start, (void *) alloc_end);
 
 	start_barebox();
 
-- 
2.17.1


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

Re: [PATCH] kvx: exclude dtb from malloc zone

[Sascha Hauer]

Hi Clement,

On Thu, Jul 09, 2020 at 10:22:04PM +0200, Clement Leger wrote:
> When dtb is provided by the FSBL (first stage bootloader), it might
> end up in the malloc zone initialized by barebox. While the dtb is
> discarded after being unflatten, if the dtb is provided right after
> barebox elf, then the malloc can overwrite it while unflattening it when
> allocating nodes.
> To avoid that, exclude the dtb from the malloc zone by checking if it
> overlaps it. If so, determine the largest zone for the allocation and
> modify the memory area to use that.

Isn't there some safe place where the dtb can be copied to? Making the
malloc area smaller seems a bit wasteful.

Sascha

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
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] kvx: exclude dtb from malloc zone

[Clément Leger]

Hi Sascha

----- On 11 Jul, 2020, at 06:41, Sascha Hauer s.hauer@pengutronix.de wrote:

> Hi Clement,
> 
> On Thu, Jul 09, 2020 at 10:22:04PM +0200, Clement Leger wrote:
>> When dtb is provided by the FSBL (first stage bootloader), it might
>> end up in the malloc zone initialized by barebox. While the dtb is
>> discarded after being unflatten, if the dtb is provided right after
>> barebox elf, then the malloc can overwrite it while unflattening it when
>> allocating nodes.
>> To avoid that, exclude the dtb from the malloc zone by checking if it
>> overlaps it. If so, determine the largest zone for the allocation and
>> modify the memory area to use that.
> 
> Isn't there some safe place where the dtb can be copied to? Making the
> malloc area smaller seems a bit wasteful.

Currently, the FSBL now always put the dtb after the barebox elf which lead
to a minimal impact on the malloc zone (~3Gib) malloc area since barebox runs
in DDR. 

We could reserve some space in barebox to recopy the dtb early after boot
before setting up the malloc. However, what size should it be to accomodate
all devices trees ? We could also copy it in smem but since the kvx barebox
port is memory agnostic (ie can run in either SMEM or DDR), I'm not sure
this is future proof.
Do you have any other idea ? 

Thanks,

Clément

> 
> Sascha
> 
> --
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 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] kvx: exclude dtb from malloc zone

[Clément Leger]

Hi Sascha,

----- On 11 Jul, 2020, at 12:19, Clément Leger cleger@kalray.eu wrote:

> Hi Sascha
> 
> ----- On 11 Jul, 2020, at 06:41, Sascha Hauer s.hauer@pengutronix.de wrote:
> 
>> Hi Clement,
>> 
>> On Thu, Jul 09, 2020 at 10:22:04PM +0200, Clement Leger wrote:
>>> When dtb is provided by the FSBL (first stage bootloader), it might
>>> end up in the malloc zone initialized by barebox. While the dtb is
>>> discarded after being unflatten, if the dtb is provided right after
>>> barebox elf, then the malloc can overwrite it while unflattening it when
>>> allocating nodes.
>>> To avoid that, exclude the dtb from the malloc zone by checking if it
>>> overlaps it. If so, determine the largest zone for the allocation and
>>> modify the memory area to use that.
>> 
>> Isn't there some safe place where the dtb can be copied to? Making the
>> malloc area smaller seems a bit wasteful.
> 
> Currently, the FSBL now always put the dtb after the barebox elf which lead
> to a minimal impact on the malloc zone (~3Gib) malloc area since barebox runs
> in DDR.
> 
> We could reserve some space in barebox to recopy the dtb early after boot
> before setting up the malloc. However, what size should it be to accomodate
> all devices trees ? We could also copy it in smem but since the kvx barebox
> port is memory agnostic (ie can run in either SMEM or DDR), I'm not sure
> this is future proof.
> Do you have any other idea ?

Do you have any additionnal comments on this one ?

Thanks,

Clément

> 
> Thanks,
> 
> Clément
> 
>> 
>> Sascha
>> 
>> --
>> Pengutronix e.K.                           |                             |
>> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
>> 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

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

Re: [PATCH] kvx: exclude dtb from malloc zone

[Sascha Hauer]

On Thu, Jul 09, 2020 at 10:22:04PM +0200, Clement Leger wrote:
> When dtb is provided by the FSBL (first stage bootloader), it might
> end up in the malloc zone initialized by barebox. While the dtb is
> discarded after being unflatten, if the dtb is provided right after
> barebox elf, then the malloc can overwrite it while unflattening it when
> allocating nodes.
> To avoid that, exclude the dtb from the malloc zone by checking if it
> overlaps it. If so, determine the largest zone for the allocation and
> modify the memory area to use that.
> 
> Signed-off-by: Clement Leger <cleger@kalray.eu>
> ---
>  arch/kvx/lib/board.c | 43 +++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 41 insertions(+), 2 deletions(-)

Applied, thanks

Sascha

> 
> diff --git a/arch/kvx/lib/board.c b/arch/kvx/lib/board.c
> index 4d6ca6983..78fa83e02 100644
> --- a/arch/kvx/lib/board.c
> +++ b/arch/kvx/lib/board.c
> @@ -103,15 +103,54 @@ err:
>  	while (1);
>  }
>  
> +/**
> + * exclude_dtb_from_alloc - Find best zone to allocate without overwriting dtb
> + *
> + * @fdt: fdt blob
> + * @alloc_start: start of allocation zone
> + * @alloc_end: end of allocation zone
> + */
> +static void exclude_dtb_from_alloc(void *fdt, u64 *alloc_start, u64 *alloc_end)
> +{
> +	const struct fdt_header *fdth = fdt;
> +	u64 fdt_start = (u64) fdt;
> +	u64 fdt_end = fdt_start + be32_to_cpu(fdth->totalsize);
> +	u64 start_size = 0, end_size = 0;
> +
> +	/*
> +	 * If the device tree is inside the malloc zone, we must exclude it to
> +	 * avoid allocating memory over it while unflattening it
> +	 */
> +	if (fdt_end < *alloc_start || fdt_start > (*alloc_end))
> +		return;
> +
> +	/* Compute the largest remaining chunk when removing the dtb */
> +	if (fdt_start >= *alloc_start)
> +		start_size = (fdt_start - *alloc_start);
> +
> +	if (fdt_end <= *alloc_end)
> +		end_size = *alloc_end - fdt_end;
> +
> +	/* Modify start/end to reflect the maximum area we found */
> +	if (start_size >= end_size)
> +		*alloc_end = fdt_start;
> +	else
> +		*alloc_start = fdt_end;
> +}
> +
>  void __noreturn kvx_start_barebox(void)
>  {
>  	u64 memsize = 0, membase = 0;
>  	u64 barebox_text_end = (u64) &__end;
> +	u64 alloc_start, alloc_end;
>  
>  	of_find_mem(boot_dtb, barebox_text_end, &membase, &memsize);
>  
> -	mem_malloc_init((void *) barebox_text_end,
> -			(void *) (membase + memsize));
> +	alloc_start = barebox_text_end;
> +	alloc_end = (membase + memsize);
> +	exclude_dtb_from_alloc(boot_dtb, &alloc_start, &alloc_end);
> +
> +	mem_malloc_init((void *) alloc_start, (void *) alloc_end);
>  
>  	start_barebox();
>  
> -- 
> 2.17.1
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
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