linux-efi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Matt Fleming <matt@codeblueprint.co.uk>
To: Ingo Molnar <mingo@kernel.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	"H . Peter Anvin" <hpa@zytor.com>
Cc: Jeffrey Hugo <jhugo@codeaurora.org>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org,
	Matt Fleming <matt@codeblueprint.co.uk>,
	Leif Lindholm <leif.lindholm@linaro.org>,
	Mark Rutland <mark.rutland@arm.com>,
	stable@vger.kernel.org
Subject: [PATCH 5/6] x86/efi: Use efi_exit_boot_services()
Date: Fri, 19 Aug 2016 21:35:03 +0100	[thread overview]
Message-ID: <1471638904-3494-6-git-send-email-matt@codeblueprint.co.uk> (raw)
In-Reply-To: <1471638904-3494-1-git-send-email-matt@codeblueprint.co.uk>

From: Jeffrey Hugo <jhugo@codeaurora.org>

The eboot code directly calls ExitBootServices.  This is inadvisable as the
UEFI spec details a complex set of errors, race conditions, and API
interactions that the caller of ExitBootServices must get correct.  The
eboot code attempts allocations after calling ExitBootSerives which is
not permitted per the spec.  The efi_exit_boot_services() helper handles
this scenario.

Signed-off-by: Jeffrey Hugo <jhugo@codeaurora.org>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: <stable@vger.kernel.org>
Signed-off-by: Matt Fleming <matt@codeblueprint.co.uk>
---
 arch/x86/boot/compressed/eboot.c | 122 +++++++++++++++++++--------------------
 1 file changed, 60 insertions(+), 62 deletions(-)

diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
index 60348c73bed4..54b5d5a9cbe5 100644
--- a/arch/x86/boot/compressed/eboot.c
+++ b/arch/x86/boot/compressed/eboot.c
@@ -1004,85 +1004,87 @@ static efi_status_t alloc_e820ext(u32 nr_desc, struct setup_data **e820ext,
 	return status;
 }
 
+struct exit_boot_struct {
+	struct boot_params *boot_params;
+	struct efi_info *efi;
+	struct setup_data *e820ext;
+	__u32 e820ext_size;
+	bool is64;
+};
+
+static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
+				   efi_boottime_memory_map_t *map,
+				   void *priv)
+{
+	static bool first = true;
+	const char *signature;
+	__u32 nr_desc;
+	efi_status_t status;
+	struct exit_boot_struct *p = priv;
+
+	if (first) {
+		nr_desc = *map->buff_size / *map->desc_size;
+		if (nr_desc > ARRAY_SIZE(p->boot_params->e820_map)) {
+			u32 nr_e820ext = nr_desc -
+					ARRAY_SIZE(p->boot_params->e820_map);
+
+			status = alloc_e820ext(nr_e820ext, &p->e820ext,
+					       &p->e820ext_size);
+			if (status != EFI_SUCCESS)
+				return status;
+		}
+		first = false;
+	}
+
+	signature = p->is64 ? EFI64_LOADER_SIGNATURE : EFI32_LOADER_SIGNATURE;
+	memcpy(&p->efi->efi_loader_signature, signature, sizeof(__u32));
+
+	p->efi->efi_systab = (unsigned long)sys_table_arg;
+	p->efi->efi_memdesc_size = *map->desc_size;
+	p->efi->efi_memdesc_version = *map->desc_ver;
+	p->efi->efi_memmap = (unsigned long)*map->map;
+	p->efi->efi_memmap_size = *map->map_size;
+
+#ifdef CONFIG_X86_64
+	p->efi->efi_systab_hi = (unsigned long)sys_table_arg >> 32;
+	p->efi->efi_memmap_hi = (unsigned long)*map->map >> 32;
+#endif
+
+	return EFI_SUCCESS;
+}
+
 static efi_status_t exit_boot(struct boot_params *boot_params,
 			      void *handle, bool is64)
 {
-	struct efi_info *efi = &boot_params->efi_info;
 	unsigned long map_sz, key, desc_size, buff_size;
 	efi_memory_desc_t *mem_map;
 	struct setup_data *e820ext;
-	const char *signature;
 	__u32 e820ext_size;
-	__u32 nr_desc, prev_nr_desc;
 	efi_status_t status;
 	__u32 desc_version;
-	bool called_exit = false;
-	u8 nr_entries;
-	int i;
 	efi_boottime_memory_map_t map;
+	struct exit_boot_struct priv;
 
-	nr_desc = 0;
-	e820ext = NULL;
-	e820ext_size = 0;
 	map.map = &mem_map;
 	map.map_size = &map_sz;
 	map.desc_size = &desc_size;
 	map.desc_ver = &desc_version;
 	map.key_ptr = &key;
 	map.buff_size = &buff_size;
+	priv.boot_params = boot_params;
+	priv.efi = &boot_params->efi_info;
+	priv.e820ext = NULL;
+	priv.e820ext_size = 0;
+	priv.is64 = is64;
 
-get_map:
-	status = efi_get_memory_map(sys_table, &map);
-
+	/* Might as well exit boot services now */
+	status = efi_exit_boot_services(sys_table, handle, &map, &priv,
+					exit_boot_func);
 	if (status != EFI_SUCCESS)
 		return status;
 
-	prev_nr_desc = nr_desc;
-	nr_desc = map_sz / desc_size;
-	if (nr_desc > prev_nr_desc &&
-	    nr_desc > ARRAY_SIZE(boot_params->e820_map)) {
-		u32 nr_e820ext = nr_desc - ARRAY_SIZE(boot_params->e820_map);
-
-		status = alloc_e820ext(nr_e820ext, &e820ext, &e820ext_size);
-		if (status != EFI_SUCCESS)
-			goto free_mem_map;
-
-		efi_call_early(free_pool, mem_map);
-		goto get_map; /* Allocated memory, get map again */
-	}
-
-	signature = is64 ? EFI64_LOADER_SIGNATURE : EFI32_LOADER_SIGNATURE;
-	memcpy(&efi->efi_loader_signature, signature, sizeof(__u32));
-
-	efi->efi_systab = (unsigned long)sys_table;
-	efi->efi_memdesc_size = desc_size;
-	efi->efi_memdesc_version = desc_version;
-	efi->efi_memmap = (unsigned long)mem_map;
-	efi->efi_memmap_size = map_sz;
-
-#ifdef CONFIG_X86_64
-	efi->efi_systab_hi = (unsigned long)sys_table >> 32;
-	efi->efi_memmap_hi = (unsigned long)mem_map >> 32;
-#endif
-
-	/* Might as well exit boot services now */
-	status = efi_call_early(exit_boot_services, handle, key);
-	if (status != EFI_SUCCESS) {
-		/*
-		 * ExitBootServices() will fail if any of the event
-		 * handlers change the memory map. In which case, we
-		 * must be prepared to retry, but only once so that
-		 * we're guaranteed to exit on repeated failures instead
-		 * of spinning forever.
-		 */
-		if (called_exit)
-			goto free_mem_map;
-
-		called_exit = true;
-		efi_call_early(free_pool, mem_map);
-		goto get_map;
-	}
-
+	e820ext = priv.e820ext;
+	e820ext_size = priv.e820ext_size;
 	/* Historic? */
 	boot_params->alt_mem_k = 32 * 1024;
 
@@ -1091,10 +1093,6 @@ get_map:
 		return status;
 
 	return EFI_SUCCESS;
-
-free_mem_map:
-	efi_call_early(free_pool, mem_map);
-	return status;
 }
 
 /*
-- 
2.7.3

  parent reply	other threads:[~2016-08-19 20:35 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-08-19 20:34 [GIT PULL 0/6] EFI urgent fixes Matt Fleming
2016-08-19 20:34 ` [PATCH 1/6] efi: Make for_each_efi_memory_desc_in_map() cope with running on Xen Matt Fleming
2016-08-19 20:35 ` [PATCH 2/6] efi/libstub: Allocate headspace in efi_get_memory_map() Matt Fleming
2016-08-22 16:37   ` Ingo Molnar
2016-08-19 20:35 ` [PATCH 3/6] efi/libstub: Introduce ExitBootServices helper Matt Fleming
     [not found]   ` <1471638904-3494-4-git-send-email-matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
2016-08-22 16:41     ` Ingo Molnar
2016-08-19 20:35 ` [PATCH 4/6] efi/libstub: Use efi_exit_boot_services() in FDT Matt Fleming
2016-08-22 16:43   ` Ingo Molnar
2016-08-19 20:35 ` Matt Fleming [this message]
2016-08-19 20:35 ` [PATCH 6/6] efi/fdt: Fix handling error value in fdt_find_uefi_params Matt Fleming
2016-08-22 16:45   ` Ingo Molnar
     [not found]     ` <20160822164508.GD11327-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2016-08-30 10:07       ` Matt Fleming
     [not found]         ` <20160830100742.GA32579-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org>
2016-08-30 10:41           ` [PATCH v2] efi: fix " Andrzej Hajda
     [not found]             ` <1472553697-27984-1-git-send-email-a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
2016-09-05 10:27               ` Matt Fleming

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1471638904-3494-6-git-send-email-matt@codeblueprint.co.uk \
    --to=matt@codeblueprint.co.uk \
    --cc=ard.biesheuvel@linaro.org \
    --cc=hpa@zytor.com \
    --cc=jhugo@codeaurora.org \
    --cc=leif.lindholm@linaro.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=mingo@kernel.org \
    --cc=stable@vger.kernel.org \
    --cc=tglx@linutronix.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).