When a payload calls our memory allocator with the exact address hint, we happily allocate memory from completely unpopulated regions. Payloads however expect this to only succeed if they would be allocating from free conventional memory.

This patch makes the logic behind those checks a bit more obvious and ensures that we always allocate from known good free conventional memory regions if we want to allocate ram.

Reported-by: Jonathan Gray jsg@jsg.id.au Signed-off-by: Alexander Graf agraf@suse.de --- lib/efi_loader/efi_memory.c | 55 ++++++++++++++++++++++++++++++++++++--------- 1 file changed, 45 insertions(+), 10 deletions(-)

diff --git a/lib/efi_loader/efi_memory.c b/lib/efi_loader/efi_memory.c index 71a3d19..64f0194 100644 --- a/lib/efi_loader/efi_memory.c +++ b/lib/efi_loader/efi_memory.c @@ -24,6 +24,10 @@ struct efi_mem_list { struct efi_mem_desc desc; };

+#define EFI_CARVE_NO_OVERLAP -1 +#define EFI_CARVE_LOOP_AGAIN -2 +#define EFI_CARVE_OVERLAPS_NONRAM -3 + /* This list contains all memory map items */ LIST_HEAD(efi_mem);

@@ -74,11 +78,11 @@ static int efi_mem_carve_out(struct efi_mem_list *map,

/* check whether we're overlapping */ if ((carve_end <= map_start) || (carve_start >= map_end)) - return 0; + return EFI_CARVE_NO_OVERLAP;

/* We're overlapping with non-RAM, warn the caller if desired */ if (overlap_only_ram && (map_desc->type != EFI_CONVENTIONAL_MEMORY)) - return -1; + return EFI_CARVE_OVERLAPS_NONRAM;

/* Sanitize carve_start and carve_end to lie within our bounds */ carve_start = max(carve_start, map_start); @@ -93,7 +97,7 @@ static int efi_mem_carve_out(struct efi_mem_list *map,

map_desc->physical_start = carve_end; map_desc->num_pages = (map_end - carve_end) >> EFI_PAGE_SHIFT; - return 1; + return (carve_end - carve_start) >> EFI_PAGE_SHIFT; }

/* @@ -113,7 +117,7 @@ static int efi_mem_carve_out(struct efi_mem_list *map, /* Shrink the map to [ map_start ... carve_start ] */ map_desc->num_pages = (carve_start - map_start) >> EFI_PAGE_SHIFT;

- return 1; + return EFI_CARVE_LOOP_AGAIN; }

uint64_t efi_add_memory_map(uint64_t start, uint64_t pages, int memory_type, @@ -121,7 +125,8 @@ uint64_t efi_add_memory_map(uint64_t start, uint64_t pages, int memory_type, { struct list_head *lhandle; struct efi_mem_list *newlist; - bool do_carving; + bool carve_again; + uint64_t carved_pages = 0;

if (!pages) return start; @@ -148,7 +153,7 @@ uint64_t efi_add_memory_map(uint64_t start, uint64_t pages, int memory_type,

/* Add our new map */ do { - do_carving = false; + carve_again = false; list_for_each(lhandle, &efi_mem) { struct efi_mem_list *lmem; int r; @@ -156,14 +161,44 @@ uint64_t efi_add_memory_map(uint64_t start, uint64_t pages, int memory_type, lmem = list_entry(lhandle, struct efi_mem_list, link); r = efi_mem_carve_out(lmem, &newlist->desc, overlap_only_ram); - if (r < 0) { + switch (r) { + case EFI_CARVE_OVERLAPS_NONRAM: + /* + * The user requested to only have RAM overlaps, + * but we hit a non-RAM region. Error out. + */ return 0; - } else if (r) { - do_carving = true; + case EFI_CARVE_NO_OVERLAP: + /* Just ignore this list entry */ + break; + case EFI_CARVE_LOOP_AGAIN: + /* + * We split an entry, but need to loop through + * the list again to actually carve it. + */ + carve_again = true; + break; + default: + /* We carved a number of pages */ + carved_pages += r; + carve_again = true; + break; + } + + if (carve_again) { + /* The list changed, we need to start over */ break; } } - } while (do_carving); + } while (carve_again); + + if (overlap_only_ram && (carved_pages != pages)) { + /* + * The payload wanted to have RAM overlaps, but we overlapped + * with an unallocated region. Error out. + */ + return 0; + }

/* Add our new map */ list_add_tail(&newlist->link, &efi_mem);