[PATCH 0/2] lmb: consider EFI memory map
As reported in Debian bug #1027176 relocation of the initrd may lead to overwriting memory used by the EFI sub-system.
Currently the available memory for images is determined via the lmb library functions. The lmb library has several shortcomings:
* It does not protect against overwriting one image with another. * The same routines to find reserved memory are called again and again.
In the long run we should move to allocating memory for images.
As an intermediate solutions let's add lmb-reservations for all EFI memory areas that are not EFI_CONVENTIONAL_MEMORY.
The variable $loadaddr of at least the vexpress_ca9x4 board collids with memory used by the EFI sub-system. Adjust $loadaddr for the vexpress boards to a sane value.
Heinrich Schuchardt (2): vexpress: adjust loadaddr lmb: consider EFI memory map
include/configs/vexpress_common.h | 1 + lib/lmb.c | 45 +++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+)
On the vexpress_ca9x4 $loadaddr points to a memory area used by the EFI sub-system. Use the same value as $kernel_addr_r which is safe.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- include/configs/vexpress_common.h | 1 + 1 file changed, 1 insertion(+)
diff --git a/include/configs/vexpress_common.h b/include/configs/vexpress_common.h index 5d773060d8..aac96d29ba 100644 --- a/include/configs/vexpress_common.h +++ b/include/configs/vexpress_common.h @@ -147,6 +147,7 @@ #include <config_distro_bootcmd.h>
#define CONFIG_EXTRA_ENV_SETTINGS \ + "loadaddr=0x60100000\0" \ "kernel_addr_r=0x60100000\0" \ "fdt_addr_r=0x60000000\0" \ "bootargs=console=tty0 console=ttyAMA0,38400n8\0" \
Add reservations for all EFI memory areas that are not EFI_CONVENTIONAL_MEMORY.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com --- lib/lmb.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)
diff --git a/lib/lmb.c b/lib/lmb.c index c599608fa3..aee2617593 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -7,7 +7,9 @@ */
#include <common.h> +#include <efi_loader.h> #include <image.h> +#include <mapmem.h> #include <lmb.h> #include <log.h> #include <malloc.h> @@ -153,6 +155,46 @@ void arch_lmb_reserve_generic(struct lmb *lmb, ulong sp, ulong end, ulong align) } }
+/** + * efi_lmb_reserve() - add reservations for EFI memory + * + * Add reservations for all EFI memory areas that are not + * EFI_CONVENTIONAL_MEMORY. + * + * @lmb: lmb environment + * Return: 0 on success, 1 on failure + */ +static __maybe_unused int efi_lmb_reserve(struct lmb *lmb) +{ + struct efi_mem_desc *memmap = NULL, *map; + efi_uintn_t i, map_size = 0; + efi_status_t ret; + + ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL); + if (ret == EFI_BUFFER_TOO_SMALL) { + map_size += sizeof(struct efi_mem_desc); /* for my own */ + ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, map_size, + (void *)&memmap); + if (ret != EFI_SUCCESS) + return 1; + ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL); + } + if (ret != EFI_SUCCESS) { + efi_free_pool(memmap); + return 1; + } + + for (i = 0, map = memmap; i < map_size / sizeof(*map); ++map, ++i) { + if (map->type != EFI_CONVENTIONAL_MEMORY) + lmb_reserve(lmb, + map_to_sysmem((void *)(uintptr_t) + map->physical_start), + map->num_pages * EFI_PAGE_SIZE); + } + efi_free_pool(memmap); + return 0; +} + static void lmb_reserve_common(struct lmb *lmb, void *fdt_blob) { arch_lmb_reserve(lmb); @@ -160,6 +202,9 @@ static void lmb_reserve_common(struct lmb *lmb, void *fdt_blob)
if (CONFIG_IS_ENABLED(OF_LIBFDT) && fdt_blob) boot_fdt_add_mem_rsv_regions(lmb, fdt_blob); + + if (CONFIG_IS_ENABLED(EFI_LOADER)) + efi_lmb_reserve(lmb); }
/* Initialize the struct, add memory and call arch/board reserve functions */
Hi Heinrich,
This looks reasonable to me as a short term solution, but I'd feel safer if someone else had a look at it
On Wed, Jan 04, 2023 at 05:26:06AM +0100, Heinrich Schuchardt wrote:
Add reservations for all EFI memory areas that are not EFI_CONVENTIONAL_MEMORY.
Signed-off-by: Heinrich Schuchardt heinrich.schuchardt@canonical.com
lib/lmb.c | 45 +++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+)
diff --git a/lib/lmb.c b/lib/lmb.c index c599608fa3..aee2617593 100644 --- a/lib/lmb.c +++ b/lib/lmb.c @@ -7,7 +7,9 @@ */
#include <common.h> +#include <efi_loader.h> #include <image.h> +#include <mapmem.h> #include <lmb.h> #include <log.h> #include <malloc.h> @@ -153,6 +155,46 @@ void arch_lmb_reserve_generic(struct lmb *lmb, ulong sp, ulong end, ulong align) } }
+/**
- efi_lmb_reserve() - add reservations for EFI memory
- Add reservations for all EFI memory areas that are not
- EFI_CONVENTIONAL_MEMORY.
- @lmb: lmb environment
- Return: 0 on success, 1 on failure
- */
+static __maybe_unused int efi_lmb_reserve(struct lmb *lmb) +{
- struct efi_mem_desc *memmap = NULL, *map;
- efi_uintn_t i, map_size = 0;
- efi_status_t ret;
- ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);
- if (ret == EFI_BUFFER_TOO_SMALL) {
map_size += sizeof(struct efi_mem_desc); /* for my own */ret = efi_allocate_pool(EFI_BOOT_SERVICES_DATA, map_size,(void *)&memmap);if (ret != EFI_SUCCESS)return 1;ret = efi_get_memory_map(&map_size, memmap, NULL, NULL, NULL);- }
- if (ret != EFI_SUCCESS) {
efi_free_pool(memmap);return 1;- }
We got a very similar piece of code in cmd/efidebug.c. I think we are better off adding a function that just calls efi_get_memory_map() and returns the map + map_size, instead of duplicating that
- for (i = 0, map = memmap; i < map_size / sizeof(*map); ++map, ++i) {
if (map->type != EFI_CONVENTIONAL_MEMORY)lmb_reserve(lmb,map_to_sysmem((void *)(uintptr_t)map->physical_start),map->num_pages * EFI_PAGE_SIZE);- }
- efi_free_pool(memmap);
- return 0;
+}
static void lmb_reserve_common(struct lmb *lmb, void *fdt_blob) { arch_lmb_reserve(lmb); @@ -160,6 +202,9 @@ static void lmb_reserve_common(struct lmb *lmb, void *fdt_blob)
if (CONFIG_IS_ENABLED(OF_LIBFDT) && fdt_blob) boot_fdt_add_mem_rsv_regions(lmb, fdt_blob);
- if (CONFIG_IS_ENABLED(EFI_LOADER))
efi_lmb_reserve(lmb);}
/* Initialize the struct, add memory and call arch/board reserve functions */
2.37.2
Other than that this looks fine
Cheers /Ilias
participants (2)
-
Heinrich Schuchardt -
Ilias Apalodimas