All of lore.kernel.org
 help / color / mirror / Atom feed
* [GIT PULL] Another EFI BGRT fix for v4.12
@ 2017-06-09  8:45 ` Ard Biesheuvel
  0 siblings, 0 replies; 4+ messages in thread
From: Ard Biesheuvel @ 2017-06-09  8:45 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Ard Biesheuvel, linux-kernel, Dave Young, Maniaxx, Matt Fleming

The following changes since commit 7425826f4f7ac60f2538b06a7f0a5d1006405159:

  efi/bgrt: Skip efi_bgrt_init() in case of non-EFI boot (2017-05-28 11:06:17 +0200)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git tags/efi-urgent

for you to fetch changes up to 0a97e704d93fe4facf2bffe3c78095b9d441df42:

  efi: fix boot panic because of invalid bgrt image address (2017-06-09 08:38:49 +0000)

----------------------------------------------------------------
Another BGRT regression fix from Dave Young, to avoid a crash on buggy
firmware that puts a bogus image address in the BGRT table.

----------------------------------------------------------------
Dave Young (1):
      efi: fix boot panic because of invalid bgrt image address

 drivers/firmware/efi/efi-bgrt.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [GIT PULL] Another EFI BGRT fix for v4.12
@ 2017-06-09  8:45 ` Ard Biesheuvel
  0 siblings, 0 replies; 4+ messages in thread
From: Ard Biesheuvel @ 2017-06-09  8:45 UTC (permalink / raw)
  To: linux-efi-u79uwXL29TY76Z2rM5mHXA, Ingo Molnar, Thomas Gleixner,
	H . Peter Anvin
  Cc: Ard Biesheuvel, linux-kernel-u79uwXL29TY76Z2rM5mHXA, Dave Young,
	Maniaxx, Matt Fleming

The following changes since commit 7425826f4f7ac60f2538b06a7f0a5d1006405159:

  efi/bgrt: Skip efi_bgrt_init() in case of non-EFI boot (2017-05-28 11:06:17 +0200)

are available in the git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/efi/efi.git tags/efi-urgent

for you to fetch changes up to 0a97e704d93fe4facf2bffe3c78095b9d441df42:

  efi: fix boot panic because of invalid bgrt image address (2017-06-09 08:38:49 +0000)

----------------------------------------------------------------
Another BGRT regression fix from Dave Young, to avoid a crash on buggy
firmware that puts a bogus image address in the BGRT table.

----------------------------------------------------------------
Dave Young (1):
      efi: fix boot panic because of invalid bgrt image address

 drivers/firmware/efi/efi-bgrt.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH] efi: fix boot panic because of invalid bgrt image address
  2017-06-09  8:45 ` Ard Biesheuvel
  (?)
@ 2017-06-09  8:45 ` Ard Biesheuvel
  2017-06-09 18:25   ` [tip:efi/urgent] efi: Fix boot panic because of invalid BGRT " tip-bot for Dave Young
  -1 siblings, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2017-06-09  8:45 UTC (permalink / raw)
  To: linux-efi, Ingo Molnar, Thomas Gleixner, H . Peter Anvin
  Cc: Dave Young, Ard Biesheuvel, linux-kernel, Matt Fleming

From: Dave Young <dyoung@redhat.com>

Maniaxx <tripleshiftone@gmail.com> reported a kernel boot failure of below:
(emulated the panic by using same invalid phys addr in code)
There are also a bug in bugzilla.kernel.org:
https://bugzilla.kernel.org/show_bug.cgi?id=195633

The reported panic happens after below commit:
7b0a911478c7 efi/x86: Move the EFI BGRT init code to early init code

The root cause is the firmware on those machines provides invalid bgrt
image addresses.

In a kernel before above commit bgrt initializes late and use ioremap
to map the image address. Ioremap validate the address, if it is not a
valid physical address ioremap just fails and returns. However in current
kernel efi bgrt initializes early and uses early_memremap which does not
validate the image address, and kernel panic happens.

According to ACPI spec the BGRT image address should fall into
EFI_BOOT_SERVICES_DATA, see the section 5.2.22.4 of below document:
http://www.uefi.org/sites/default/files/resources/ACPI_6_1.pdf

Fix this issue by validating the image address in efi_bgrt_init(). If the
image address does not fall into any EFI_BOOT_SERVICES_DATA areas we just
bail out.

  BUG: unable to handle kernel paging request at ffffffffff280001
  IP: efi_bgrt_init+0xfb/0x153
  ...
  Call Trace:
   ? bgrt_init+0xbc/0xbc
   acpi_parse_bgrt+0xe/0x12
   acpi_table_parse+0x89/0xb8
   acpi_boot_init+0x445/0x4e2
   ? acpi_parse_x2apic+0x79/0x79
   ? dmi_ignore_irq0_timer_override+0x33/0x33
   setup_arch+0xb63/0xc82
   ? early_idt_handler_array+0x120/0x120
   start_kernel+0xb7/0x443
   ? early_idt_handler_array+0x120/0x120
   x86_64_start_reservations+0x29/0x2b
   x86_64_start_kernel+0x154/0x177
   secondary_startup_64+0x9f/0x9f

Fixes: 7b0a911478c7 ("efi/x86: Move the EFI BGRT init code to early init code")
Reported-by: Maniaxx <tripleshiftone@gmail.com>
Signed-off-by: Dave Young <dyoung@redhat.com>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 drivers/firmware/efi/efi-bgrt.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/efi-bgrt.c b/drivers/firmware/efi/efi-bgrt.c
index 8bf27323f7a3..b58233e4ed71 100644
--- a/drivers/firmware/efi/efi-bgrt.c
+++ b/drivers/firmware/efi/efi-bgrt.c
@@ -27,6 +27,26 @@ struct bmp_header {
 	u32 size;
 } __packed;
 
+static bool efi_bgrt_addr_valid(u64 addr)
+{
+	efi_memory_desc_t *md;
+
+	for_each_efi_memory_desc(md) {
+		u64 size;
+		u64 end;
+
+		if (md->type != EFI_BOOT_SERVICES_DATA)
+			continue;
+
+		size = md->num_pages << EFI_PAGE_SHIFT;
+		end = md->phys_addr + size;
+		if (addr >= md->phys_addr && addr < end)
+			return true;
+	}
+
+	return false;
+}
+
 void __init efi_bgrt_init(struct acpi_table_header *table)
 {
 	void *image;
@@ -36,7 +56,7 @@ void __init efi_bgrt_init(struct acpi_table_header *table)
 	if (acpi_disabled)
 		return;
 
-	if (!efi_enabled(EFI_BOOT))
+	if (!efi_enabled(EFI_MEMMAP))
 		return;
 
 	if (table->length < sizeof(bgrt_tab)) {
@@ -65,6 +85,10 @@ void __init efi_bgrt_init(struct acpi_table_header *table)
 		goto out;
 	}
 
+	if (!efi_bgrt_addr_valid(bgrt->image_address)) {
+		pr_notice("Ignoring BGRT: invalid image address\n");
+		goto out;
+	}
 	image = early_memremap(bgrt->image_address, sizeof(bmp_header));
 	if (!image) {
 		pr_notice("Ignoring BGRT: failed to map image header memory\n");
-- 
2.9.3

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [tip:efi/urgent] efi: Fix boot panic because of invalid BGRT image address
  2017-06-09  8:45 ` [PATCH] efi: fix boot panic because of invalid bgrt image address Ard Biesheuvel
@ 2017-06-09 18:25   ` tip-bot for Dave Young
  0 siblings, 0 replies; 4+ messages in thread
From: tip-bot for Dave Young @ 2017-06-09 18:25 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: tripleshiftone, linux-kernel, matt, dyoung, ard.biesheuvel, tglx,
	torvalds, mingo, hpa, peterz

Commit-ID:  792ef14df5c585c19b2831673a077504a09e5203
Gitweb:     http://git.kernel.org/tip/792ef14df5c585c19b2831673a077504a09e5203
Author:     Dave Young <dyoung@redhat.com>
AuthorDate: Fri, 9 Jun 2017 08:45:58 +0000
Committer:  Ingo Molnar <mingo@kernel.org>
CommitDate: Fri, 9 Jun 2017 14:50:11 +0200

efi: Fix boot panic because of invalid BGRT image address

Maniaxx reported a kernel boot crash in the EFI code, which I emulated
by using same invalid phys addr in code:

  BUG: unable to handle kernel paging request at ffffffffff280001
  IP: efi_bgrt_init+0xfb/0x153
  ...
  Call Trace:
   ? bgrt_init+0xbc/0xbc
   acpi_parse_bgrt+0xe/0x12
   acpi_table_parse+0x89/0xb8
   acpi_boot_init+0x445/0x4e2
   ? acpi_parse_x2apic+0x79/0x79
   ? dmi_ignore_irq0_timer_override+0x33/0x33
   setup_arch+0xb63/0xc82
   ? early_idt_handler_array+0x120/0x120
   start_kernel+0xb7/0x443
   ? early_idt_handler_array+0x120/0x120
   x86_64_start_reservations+0x29/0x2b
   x86_64_start_kernel+0x154/0x177
   secondary_startup_64+0x9f/0x9f

There is also a similar bug filed in bugzilla.kernel.org:

  https://bugzilla.kernel.org/show_bug.cgi?id=195633

The crash is caused by this commit:

  7b0a911478c7 efi/x86: Move the EFI BGRT init code to early init code

The root cause is the firmware on those machines provides invalid BGRT
image addresses.

In a kernel before above commit BGRT initializes late and uses ioremap()
to map the image address. Ioremap validates the address, if it is not a
valid physical address ioremap() just fails and returns. However in current
kernel EFI BGRT initializes early and uses early_memremap() which does not
validate the image address, and kernel panic happens.

According to ACPI spec the BGRT image address should fall into
EFI_BOOT_SERVICES_DATA, see the section 5.2.22.4 of below document:

  http://www.uefi.org/sites/default/files/resources/ACPI_6_1.pdf

Fix this issue by validating the image address in efi_bgrt_init(). If the
image address does not fall into any EFI_BOOT_SERVICES_DATA areas we just
bail out with a warning message.

Reported-by: Maniaxx <tripleshiftone@gmail.com>
Signed-off-by: Dave Young <dyoung@redhat.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Matt Fleming <matt@codeblueprint.co.uk>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-efi@vger.kernel.org
Fixes: 7b0a911478c7 ("efi/x86: Move the EFI BGRT init code to early init code")
Link: http://lkml.kernel.org/r/20170609084558.26766-2-ard.biesheuvel@linaro.org
Signed-off-by: Ingo Molnar <mingo@kernel.org>
---
 drivers/firmware/efi/efi-bgrt.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/efi-bgrt.c b/drivers/firmware/efi/efi-bgrt.c
index 8bf2732..b58233e 100644
--- a/drivers/firmware/efi/efi-bgrt.c
+++ b/drivers/firmware/efi/efi-bgrt.c
@@ -27,6 +27,26 @@ struct bmp_header {
 	u32 size;
 } __packed;
 
+static bool efi_bgrt_addr_valid(u64 addr)
+{
+	efi_memory_desc_t *md;
+
+	for_each_efi_memory_desc(md) {
+		u64 size;
+		u64 end;
+
+		if (md->type != EFI_BOOT_SERVICES_DATA)
+			continue;
+
+		size = md->num_pages << EFI_PAGE_SHIFT;
+		end = md->phys_addr + size;
+		if (addr >= md->phys_addr && addr < end)
+			return true;
+	}
+
+	return false;
+}
+
 void __init efi_bgrt_init(struct acpi_table_header *table)
 {
 	void *image;
@@ -36,7 +56,7 @@ void __init efi_bgrt_init(struct acpi_table_header *table)
 	if (acpi_disabled)
 		return;
 
-	if (!efi_enabled(EFI_BOOT))
+	if (!efi_enabled(EFI_MEMMAP))
 		return;
 
 	if (table->length < sizeof(bgrt_tab)) {
@@ -65,6 +85,10 @@ void __init efi_bgrt_init(struct acpi_table_header *table)
 		goto out;
 	}
 
+	if (!efi_bgrt_addr_valid(bgrt->image_address)) {
+		pr_notice("Ignoring BGRT: invalid image address\n");
+		goto out;
+	}
 	image = early_memremap(bgrt->image_address, sizeof(bmp_header));
 	if (!image) {
 		pr_notice("Ignoring BGRT: failed to map image header memory\n");

^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2017-06-09 18:31 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-06-09  8:45 [GIT PULL] Another EFI BGRT fix for v4.12 Ard Biesheuvel
2017-06-09  8:45 ` Ard Biesheuvel
2017-06-09  8:45 ` [PATCH] efi: fix boot panic because of invalid bgrt image address Ard Biesheuvel
2017-06-09 18:25   ` [tip:efi/urgent] efi: Fix boot panic because of invalid BGRT " tip-bot for Dave Young

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.