All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
To: linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org,
	james.morse-5wv7dgnIgG8@public.gmane.org,
	riku.voipio-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org,
	Ard Biesheuvel
	<ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Subject: [PATCH] efi: fdt: avoid FDT manipulation after ExitBootServices()
Date: Wed,  1 Feb 2017 13:11:33 +0000	[thread overview]
Message-ID: <1485954693-25750-1-git-send-email-ard.biesheuvel@linaro.org> (raw)

Some AArch64 UEFI implementations disable the MMU in ExitBootServices(),
after which unaligned accesses to RAM are no longer supported.

Commit abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the
kernel") fixed an issue in the memory map handling of the stub FDT code,
but inadvertently created an issue with such firmwares, by moving some
of the FDT manipulation to after the invocation of ExitBootServices().
However, the stub's libfdt implementation uses the ordinary, accelerated
string functions, which rely on hardware handling of unaligned accesses,
resulting in alignment faults when executed with the MMU off.

Whether disabling the MMU in ExitBootServices() complies with the UEFI
spec is unclear, but it is a reality we have to deal with, given that
it wasn't a problem before commit abfb7b686a3e was applied.

So fix the situation by moving the update_fdt_memmap() into the callback
invoked by efi_exit_boot_services() right before it hits the button.

Fixes: abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the kernel")
Signed-off-by: Ard Biesheuvel <ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
 drivers/firmware/efi/libstub/fdt.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index 921dfa047202..260c4b4b492e 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -187,6 +187,7 @@ static efi_status_t update_fdt_memmap(void *fdt, struct efi_boot_memmap *map)
 struct exit_boot_struct {
 	efi_memory_desc_t *runtime_map;
 	int *runtime_entry_count;
+	void *new_fdt_addr;
 };
 
 static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
@@ -202,7 +203,7 @@ static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
 	efi_get_virtmap(*map->map, *map->map_size, *map->desc_size,
 			p->runtime_map, p->runtime_entry_count);
 
-	return EFI_SUCCESS;
+	return update_fdt_memmap(p->new_fdt_addr, map);
 }
 
 /*
@@ -300,22 +301,13 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
 
 	priv.runtime_map = runtime_map;
 	priv.runtime_entry_count = &runtime_entry_count;
+	priv.new_fdt_addr = (void *)*new_fdt_addr;
 	status = efi_exit_boot_services(sys_table, handle, &map, &priv,
 					exit_boot_func);
 
 	if (status == EFI_SUCCESS) {
 		efi_set_virtual_address_map_t *svam;
 
-		status = update_fdt_memmap((void *)*new_fdt_addr, &map);
-		if (status != EFI_SUCCESS) {
-			/*
-			 * The kernel won't get far without the memory map, but
-			 * may still be able to print something meaningful so
-			 * return success here.
-			 */
-			return EFI_SUCCESS;
-		}
-
 		/* Install the new virtual address map */
 		svam = sys_table->runtime->set_virtual_address_map;
 		status = svam(runtime_entry_count * desc_size, desc_size,
-- 
2.7.4

WARNING: multiple messages have this Message-ID (diff)
From: ard.biesheuvel@linaro.org (Ard Biesheuvel)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH] efi: fdt: avoid FDT manipulation after ExitBootServices()
Date: Wed,  1 Feb 2017 13:11:33 +0000	[thread overview]
Message-ID: <1485954693-25750-1-git-send-email-ard.biesheuvel@linaro.org> (raw)

Some AArch64 UEFI implementations disable the MMU in ExitBootServices(),
after which unaligned accesses to RAM are no longer supported.

Commit abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the
kernel") fixed an issue in the memory map handling of the stub FDT code,
but inadvertently created an issue with such firmwares, by moving some
of the FDT manipulation to after the invocation of ExitBootServices().
However, the stub's libfdt implementation uses the ordinary, accelerated
string functions, which rely on hardware handling of unaligned accesses,
resulting in alignment faults when executed with the MMU off.

Whether disabling the MMU in ExitBootServices() complies with the UEFI
spec is unclear, but it is a reality we have to deal with, given that
it wasn't a problem before commit abfb7b686a3e was applied.

So fix the situation by moving the update_fdt_memmap() into the callback
invoked by efi_exit_boot_services() right before it hits the button.

Fixes: abfb7b686a3e ("efi/libstub/arm*: Pass latest memory map to the kernel")
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/libstub/fdt.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
index 921dfa047202..260c4b4b492e 100644
--- a/drivers/firmware/efi/libstub/fdt.c
+++ b/drivers/firmware/efi/libstub/fdt.c
@@ -187,6 +187,7 @@ static efi_status_t update_fdt_memmap(void *fdt, struct efi_boot_memmap *map)
 struct exit_boot_struct {
 	efi_memory_desc_t *runtime_map;
 	int *runtime_entry_count;
+	void *new_fdt_addr;
 };
 
 static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
@@ -202,7 +203,7 @@ static efi_status_t exit_boot_func(efi_system_table_t *sys_table_arg,
 	efi_get_virtmap(*map->map, *map->map_size, *map->desc_size,
 			p->runtime_map, p->runtime_entry_count);
 
-	return EFI_SUCCESS;
+	return update_fdt_memmap(p->new_fdt_addr, map);
 }
 
 /*
@@ -300,22 +301,13 @@ efi_status_t allocate_new_fdt_and_exit_boot(efi_system_table_t *sys_table,
 
 	priv.runtime_map = runtime_map;
 	priv.runtime_entry_count = &runtime_entry_count;
+	priv.new_fdt_addr = (void *)*new_fdt_addr;
 	status = efi_exit_boot_services(sys_table, handle, &map, &priv,
 					exit_boot_func);
 
 	if (status == EFI_SUCCESS) {
 		efi_set_virtual_address_map_t *svam;
 
-		status = update_fdt_memmap((void *)*new_fdt_addr, &map);
-		if (status != EFI_SUCCESS) {
-			/*
-			 * The kernel won't get far without the memory map, but
-			 * may still be able to print something meaningful so
-			 * return success here.
-			 */
-			return EFI_SUCCESS;
-		}
-
 		/* Install the new virtual address map */
 		svam = sys_table->runtime->set_virtual_address_map;
 		status = svam(runtime_entry_count * desc_size, desc_size,
-- 
2.7.4

             reply	other threads:[~2017-02-01 13:11 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-01 13:11 Ard Biesheuvel [this message]
2017-02-01 13:11 ` [PATCH] efi: fdt: avoid FDT manipulation after ExitBootServices() Ard Biesheuvel
     [not found] ` <1485954693-25750-1-git-send-email-ard.biesheuvel-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
2017-02-01 13:39   ` Leif Lindholm
2017-02-01 13:39     ` Leif Lindholm
     [not found]     ` <CAF7UmSyYvmo8k910mJEAYhaOJy9j_bpXw6f+Cj=cS_m_iYiCCQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-02-01 13:41       ` Ard Biesheuvel
2017-02-01 13:41         ` Ard Biesheuvel
2017-02-01 14:08   ` Mark Rutland
2017-02-01 14:08     ` Mark Rutland
2017-02-01 14:46   ` Riku Voipio
2017-02-01 14:46     ` Riku Voipio
2017-02-01 17:45 [GIT PULL] EFI urgent fix for v4.10 Ard Biesheuvel
2017-02-01 17:45 ` [PATCH] efi: fdt: avoid FDT manipulation after ExitBootServices() Ard Biesheuvel
2017-02-01 17:45   ` Ard Biesheuvel
2017-02-01 17:49   ` Leif Lindholm
2017-02-01 17:49     ` Leif Lindholm
2017-02-01 19:03   ` Ingo Molnar
2017-02-01 19:03     ` Ingo Molnar
2017-02-01 19:03     ` Ingo Molnar
2017-02-01 19:28     ` Ard Biesheuvel
2017-02-01 19:28       ` Ard Biesheuvel
2017-02-01 19:28       ` Ard Biesheuvel
2017-02-01 20:16       ` Ingo Molnar
2017-02-01 20:16         ` Ingo Molnar
2017-02-01 20:16         ` Ingo Molnar
2017-02-01 21:08   ` Jeffrey Hugo
2017-02-01 21:08     ` Jeffrey Hugo
2017-02-01 21:08     ` Jeffrey Hugo
2017-02-01 22:59     ` Jeffrey Hugo
2017-02-01 22:59       ` Jeffrey Hugo
2017-02-01 22:59       ` Jeffrey Hugo
2017-02-02 16:14       ` Ard Biesheuvel
2017-02-02 16:14         ` Ard Biesheuvel
2017-02-02 16:14         ` Ard Biesheuvel

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=1485954693-25750-1-git-send-email-ard.biesheuvel@linaro.org \
    --to=ard.biesheuvel-qsej5fyqhm4dnm+yrofe0a@public.gmane.org \
    --cc=james.morse-5wv7dgnIgG8@public.gmane.org \
    --cc=leif.lindholm-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    --cc=linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org \
    --cc=linux-efi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=matt-mF/unelCI9GS6iBeEJttW/XRex20P6io@public.gmane.org \
    --cc=riku.voipio-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.