mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH] kvx: exclude dtb from malloc zone
@ 2020-07-09 20:22 Clement Leger
  2020-07-11  4:41 ` Sascha Hauer
  2020-08-11  7:34 ` Sascha Hauer
  0 siblings, 2 replies; 5+ messages in thread
From: Clement Leger @ 2020-07-09 20:22 UTC (permalink / raw)
  To: Sascha Hauer, barebox; +Cc: 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

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

* Re: [PATCH] kvx: exclude dtb from malloc zone
  2020-07-09 20:22 [PATCH] kvx: exclude dtb from malloc zone Clement Leger
@ 2020-07-11  4:41 ` Sascha Hauer
  2020-07-11 10:19   ` Clément Leger
  2020-08-11  7:34 ` Sascha Hauer
  1 sibling, 1 reply; 5+ messages in thread
From: Sascha Hauer @ 2020-07-11  4:41 UTC (permalink / raw)
  To: Clement Leger; +Cc: barebox

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

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

* Re: [PATCH] kvx: exclude dtb from malloc zone
  2020-07-11  4:41 ` Sascha Hauer
@ 2020-07-11 10:19   ` Clément Leger
  2020-08-03  6:51     ` Clément Leger
  0 siblings, 1 reply; 5+ messages in thread
From: Clément Leger @ 2020-07-11 10:19 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

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

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

* Re: [PATCH] kvx: exclude dtb from malloc zone
  2020-07-11 10:19   ` Clément Leger
@ 2020-08-03  6:51     ` Clément Leger
  0 siblings, 0 replies; 5+ messages in thread
From: Clément Leger @ 2020-08-03  6:51 UTC (permalink / raw)
  To: Sascha Hauer; +Cc: Barebox List

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

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

* Re: [PATCH] kvx: exclude dtb from malloc zone
  2020-07-09 20:22 [PATCH] kvx: exclude dtb from malloc zone Clement Leger
  2020-07-11  4:41 ` Sascha Hauer
@ 2020-08-11  7:34 ` Sascha Hauer
  1 sibling, 0 replies; 5+ messages in thread
From: Sascha Hauer @ 2020-08-11  7:34 UTC (permalink / raw)
  To: Clement Leger; +Cc: barebox

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

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

end of thread, other threads:[~2020-08-11  7:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-07-09 20:22 [PATCH] kvx: exclude dtb from malloc zone Clement Leger
2020-07-11  4:41 ` Sascha Hauer
2020-07-11 10:19   ` Clément Leger
2020-08-03  6:51     ` Clément Leger
2020-08-11  7:34 ` Sascha Hauer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox