public inbox for u-boot@lists.denx.de
 help / color / mirror / Atom feed
* [PATCH v2 1/1] cmd: bootefi: allocate device-tree copy from high memory
@ 2023-02-23 19:27 Heinrich Schuchardt
  2023-02-23 19:27 ` Heinrich Schuchardt
  0 siblings, 1 reply; 3+ messages in thread
From: Heinrich Schuchardt @ 2023-02-23 19:27 UTC (permalink / raw)
  To: Ilias Apalodimas; +Cc: u-boot, Alexandre Ghiti, Heinrich Schuchardt

The bootefi command creates a copy of the device-tree within the first
127 MiB of memory. This may lead to overwriting previously loaded binaries
(e.g. kernel, initrd).

Linux EFI stub itself copies U-Boot's copy of the device-tree. This means
there is not restriction for U-Boot to place the device-tree copy to any
address. (Restrictions existed for 32bit ARM before Linux commit
7a1be318f579 ("ARM: 9012/1: move device tree mapping out of linear region")
for legacy booting.

Reported-by: Alexandre Ghiti <alexghiti@rivosinc.com>
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Tested-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
v2:
	remove superfluous comment
---
 cmd/bootefi.c | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 6618335ddf..8aa15a64c8 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -204,25 +204,12 @@ static efi_status_t copy_fdt(void **fdtp)
 	fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000);
 	fdt_size = fdt_pages << EFI_PAGE_SHIFT;
 
-	/*
-	 * Safe fdt location is at 127 MiB.
-	 * On the sandbox convert from the sandbox address space.
-	 */
-	new_fdt_addr = (uintptr_t)map_sysmem(fdt_ram_start + 0x7f00000 +
-					     fdt_size, 0);
-	ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
+	ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
 				 EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
 				 &new_fdt_addr);
 	if (ret != EFI_SUCCESS) {
-		/* If we can't put it there, put it somewhere */
-		new_fdt_addr = (ulong)memalign(EFI_PAGE_SIZE, fdt_size);
-		ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
-					 EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
-					 &new_fdt_addr);
-		if (ret != EFI_SUCCESS) {
-			log_err("ERROR: Failed to reserve space for FDT\n");
-			goto done;
-		}
+		log_err("ERROR: Failed to reserve space for FDT\n");
+		goto done;
 	}
 	new_fdt = (void *)(uintptr_t)new_fdt_addr;
 	memcpy(new_fdt, fdt, fdt_totalsize(fdt));
-- 
2.38.1


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

* [PATCH v2 1/1] cmd: bootefi: allocate device-tree copy from high memory
  2023-02-23 19:27 [PATCH v2 1/1] cmd: bootefi: allocate device-tree copy from high memory Heinrich Schuchardt
@ 2023-02-23 19:27 ` Heinrich Schuchardt
  2023-02-23 20:02   ` Ilias Apalodimas
  0 siblings, 1 reply; 3+ messages in thread
From: Heinrich Schuchardt @ 2023-02-23 19:27 UTC (permalink / raw)
  To: Ilias Apalodimas; +Cc: u-boot, Alexandre Ghiti, Heinrich Schuchardt

The bootefi command creates a copy of the device-tree within the first
127 MiB of memory. This may lead to overwriting previously loaded binaries
(e.g. kernel, initrd).

Linux EFI stub itself copies U-Boot's copy of the device-tree. This means
there is not restriction for U-Boot to place the device-tree copy to any
address. (Restrictions existed for 32bit ARM before Linux commit
7a1be318f579 ("ARM: 9012/1: move device tree mapping out of linear region")
for legacy booting.

Reported-by: Alexandre Ghiti <alexghiti@rivosinc.com>
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Tested-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
v2:
	remove superfluous comment
---
 cmd/bootefi.c | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/cmd/bootefi.c b/cmd/bootefi.c
index 6618335ddf..8aa15a64c8 100644
--- a/cmd/bootefi.c
+++ b/cmd/bootefi.c
@@ -204,25 +204,12 @@ static efi_status_t copy_fdt(void **fdtp)
 	fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000);
 	fdt_size = fdt_pages << EFI_PAGE_SHIFT;
 
-	/*
-	 * Safe fdt location is at 127 MiB.
-	 * On the sandbox convert from the sandbox address space.
-	 */
-	new_fdt_addr = (uintptr_t)map_sysmem(fdt_ram_start + 0x7f00000 +
-					     fdt_size, 0);
-	ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
+	ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
 				 EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
 				 &new_fdt_addr);
 	if (ret != EFI_SUCCESS) {
-		/* If we can't put it there, put it somewhere */
-		new_fdt_addr = (ulong)memalign(EFI_PAGE_SIZE, fdt_size);
-		ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
-					 EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
-					 &new_fdt_addr);
-		if (ret != EFI_SUCCESS) {
-			log_err("ERROR: Failed to reserve space for FDT\n");
-			goto done;
-		}
+		log_err("ERROR: Failed to reserve space for FDT\n");
+		goto done;
 	}
 	new_fdt = (void *)(uintptr_t)new_fdt_addr;
 	memcpy(new_fdt, fdt, fdt_totalsize(fdt));
-- 
2.38.1


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

* Re: [PATCH v2 1/1] cmd: bootefi: allocate device-tree copy from high memory
  2023-02-23 19:27 ` Heinrich Schuchardt
@ 2023-02-23 20:02   ` Ilias Apalodimas
  0 siblings, 0 replies; 3+ messages in thread
From: Ilias Apalodimas @ 2023-02-23 20:02 UTC (permalink / raw)
  To: Heinrich Schuchardt; +Cc: u-boot, Alexandre Ghiti

Hi Heinrich,

On Thu, Feb 23, 2023 at 08:27:39PM +0100, Heinrich Schuchardt wrote:
> The bootefi command creates a copy of the device-tree within the first
> 127 MiB of memory. This may lead to overwriting previously loaded binaries
> (e.g. kernel, initrd).
>
> Linux EFI stub itself copies U-Boot's copy of the device-tree. This means
> there is not restriction for U-Boot to place the device-tree copy to any
> address. (Restrictions existed for 32bit ARM before Linux commit
> 7a1be318f579 ("ARM: 9012/1: move device tree mapping out of linear region")
> for legacy booting.
>
> Reported-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
> Tested-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> ---
> v2:
> 	remove superfluous comment
> ---
>  cmd/bootefi.c | 19 +++----------------
>  1 file changed, 3 insertions(+), 16 deletions(-)
>
> diff --git a/cmd/bootefi.c b/cmd/bootefi.c
> index 6618335ddf..8aa15a64c8 100644
> --- a/cmd/bootefi.c
> +++ b/cmd/bootefi.c
> @@ -204,25 +204,12 @@ static efi_status_t copy_fdt(void **fdtp)
>  	fdt_pages = efi_size_in_pages(fdt_totalsize(fdt) + 0x3000);
>  	fdt_size = fdt_pages << EFI_PAGE_SHIFT;
>
> -	/*
> -	 * Safe fdt location is at 127 MiB.
> -	 * On the sandbox convert from the sandbox address space.
> -	 */
> -	new_fdt_addr = (uintptr_t)map_sysmem(fdt_ram_start + 0x7f00000 +
> -					     fdt_size, 0);
> -	ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
> +	ret = efi_allocate_pages(EFI_ALLOCATE_ANY_PAGES,
>  				 EFI_ACPI_RECLAIM_MEMORY, fdt_pages,

>  				 &new_fdt_addr);
>  	if (ret != EFI_SUCCESS) {
> -		/* If we can't put it there, put it somewhere */
> -		new_fdt_addr = (ulong)memalign(EFI_PAGE_SIZE, fdt_size);
> -		ret = efi_allocate_pages(EFI_ALLOCATE_MAX_ADDRESS,
> -					 EFI_ACPI_RECLAIM_MEMORY, fdt_pages,
> -					 &new_fdt_addr);
> -		if (ret != EFI_SUCCESS) {
> -			log_err("ERROR: Failed to reserve space for FDT\n");
> -			goto done;
> -		}
> +		log_err("ERROR: Failed to reserve space for FDT\n");
> +		goto done;
>  	}
>  	new_fdt = (void *)(uintptr_t)new_fdt_addr;
>  	memcpy(new_fdt, fdt, fdt_totalsize(fdt));
> --
> 2.38.1
>

Reviewed-by: Ilias Apalodimas <ilias.apalodimas@linaro.org>


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

end of thread, other threads:[~2023-02-23 20:02 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-23 19:27 [PATCH v2 1/1] cmd: bootefi: allocate device-tree copy from high memory Heinrich Schuchardt
2023-02-23 19:27 ` Heinrich Schuchardt
2023-02-23 20:02   ` Ilias Apalodimas

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