On Thu, 06 Jul, at 08:31:07AM, Naoya Horiguchi wrote:
>
> KASLR chooses kernel location from E820_TYPE_RAM regions by walking over
> e820 entries now. E820_TYPE_RAM includes EFI_BOOT_SERVICES_CODE and
> EFI_BOOT_SERVICES_DATA, so those regions can be the target. According to
> UEFI spec, all memory regions marked as EfiBootServicesCode and
> EfiBootServicesData are available for free memory after the first call
> of ExitBootServices(). So such regions should be usable for kernel on
> spec basis.
>
> In x86, however, we have some workaround for broken firmware, where we
> keep such regions reserved until SetVirtualAddressMap() is done.
> See the following code in should_map_region():
>
> static bool should_map_region(efi_memory_desc_t *md)
> {
> ...
> /*
> * Map boot services regions as a workaround for buggy
> * firmware that accesses them even when they shouldn't.
> *
> * See efi_{reserve,free}_boot_services().
> */
> if (md->type == EFI_BOOT_SERVICES_CODE ||
> md->type == EFI_BOOT_SERVICES_DATA)
> return false;
>
> This workaround suppressed a boot crash, but potential issues still
> remain because no one prevents the regions from overlapping with kernel
> image by KASLR.
>
> So let's make sure that EFI_BOOT_SERVICES_{CODE|DATA} regions are never
> chosen as kernel memory for the workaround to work fine.
>
> Signed-off-by: Naoya Horiguchi <n-horigu...@ah.jp.nec.com>
> ---
> arch/x86/boot/compressed/kaslr.c | 41
> +++++++++++++++++++++++++++++++---------
> 1 file changed, 32 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/kaslr.c
> b/arch/x86/boot/compressed/kaslr.c
> index 94f08fd375ae..f43fed0441a6 100644
> --- a/arch/x86/boot/compressed/kaslr.c
> +++ b/arch/x86/boot/compressed/kaslr.c
> @@ -563,7 +563,8 @@ static void process_mem_region(struct mem_vector *entry,
> /* Marks if efi mirror regions have been found and handled. */
> static bool efi_mirror_found;
>
> -static void process_efi_entry(unsigned long minimum, unsigned long
> image_size)
> +/* Returns true if we really enter efi memmap walk, otherwise returns false.
> */
> +static bool process_efi_entry(unsigned long minimum, unsigned long
> image_size)
> {
> struct efi_info *e = &boot_params->efi_info;
> struct mem_vector region;
> @@ -577,13 +578,13 @@ static void process_efi_entry(unsigned long minimum,
> unsigned long image_size)
> signature = (char *)&boot_params->efi_info.efi_loader_signature;
> if (strncmp(signature, EFI32_LOADER_SIGNATURE, 4) &&
> strncmp(signature, EFI64_LOADER_SIGNATURE, 4))
> - return;
> + return false;
>
> #ifdef CONFIG_X86_32
> /* Can't handle data above 4GB at this time */
> if (e->efi_memmap_hi) {
> warn("Memory map is above 4GB, EFI should be disabled.\n");
> - return;
> + return false;
> }
> pmap = e->efi_memmap;
> #else
> @@ -593,13 +594,36 @@ static void process_efi_entry(unsigned long minimum,
> unsigned long image_size)
> nr_desc = e->efi_memmap_size / e->efi_memdesc_size;
> for (i = 0; i < nr_desc; i++) {
> md = (efi_memory_desc_t *)(pmap + (i * e->efi_memdesc_size));
> - if (md->attribute & EFI_MEMORY_MORE_RELIABLE) {
> - region.start = md->phys_addr;
> - region.size = md->num_pages << EFI_PAGE_SHIFT;
> - process_mem_region(®ion, minimum, image_size);
> + if (md->attribute & EFI_MEMORY_MORE_RELIABLE)
> efi_mirror_found = true;
> + }
> +
> + for (i = 0; i < nr_desc; i++) {
> + md = (efi_memory_desc_t *)(pmap + (i * e->efi_memdesc_size));
> +
> + /*
> + * EFI_BOOT_SERVICES_{CODE|DATA} are avoided because boot
> + * services regions could be accessed after ExitBootServices()
> + * due to the workaround for buggy firmware.
> + */
> + if (!(md->type == EFI_LOADER_CODE ||
> + md->type == EFI_LOADER_DATA ||
> + md->type == EFI_CONVENTIONAL_MEMORY))
> + continue;
Wouldn't it make more sense to *only* use EFI_CONVENTIONAL_MEMORY?
You can't re-use EFI_LOADER_* regions because the kaslr code is run so
early in boot that you've no idea if data the kernel will need is in
those EFI_LOADER_* regions.
For example, we pass struct setup_data objects inside of
EFI_LOADER_DATA regions.
