All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Williams <dan.j.williams@intel.com>
To: mingo@redhat.com
Cc: Taku Izumi <izumi.taku@jp.fujitsu.com>,
	Michael Weiser <michael@weiser.dinsnail.net>,
	Dave Young <dyoung@redhat.com>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@kernel.org>,
	linux-kernel@vger.kernel.org, linux-efi@vger.kernel.org,
	kexec@lists.infradead.org
Subject: [PATCH] efi: Fix handling of multiple contiguous efi_fake_mem= entries
Date: Mon, 30 Dec 2019 11:58:23 -0800	[thread overview]
Message-ID: <157773590338.4153451.5898675419563883883.stgit@dwillia2-desk3.amr.corp.intel.com> (raw)

A recent test of efi_fake_mem=4G@9G:0x40000,4G@13G:0x40000 crashed with
a signature of:

    BUG: unable to handle page fault for address: ffffffffff281000
    [..]
    RIP: 0010:efi_memmap_insert+0x11d/0x191
    [..]
    Call Trace:
     ? bgrt_init+0xbe/0xbe
     ? efi_arch_mem_reserve+0x1cb/0x228
     ? acpi_parse_bgrt+0xa/0xd
     ? acpi_table_parse+0x86/0xb8
     ? acpi_boot_init+0x494/0x4e3
     ? acpi_parse_x2apic+0x87/0x87
     ? setup_acpi_sci+0xa2/0xa2
     ? setup_arch+0x8db/0x9e1
     ? start_kernel+0x6a/0x547
     ? secondary_startup_64+0xb6/0xc0

efi_memmap_insert() is attempting to insert entries past the end of the
new map. This condition is setup by efi_fake_mem() leaking empty entries
to the end of memory map, and then efi_arch_mem_reserve() trips over the
bad entry when attempting an additional efi_memmap_insert(). The empty
entry causes efi_memmap_insert() to attempt more memmap splits / copies
than efi_memmap_split_count() accounted for when sizing the new map.

Update efi_fake_memmap() to cleanup lagging empty entries.

This change is related to commit af1648984828 "x86/efi: Update e820 with
reserved EFI boot services data to fix kexec breakage" since that
introduces more occurrences where efi_memmap_insert() is invoked after
an efi_fake_mem= configuration has been parsed. Previously the side
effects of vestigial empty entries were benign, but with commit
af1648984828 that follow-on efi_memmap_insert() invocation triggers the
above crash signature.

Fixes: 0f96a99dab36 ("efi: Add 'efi_fake_mem' boot option")
Fixes: af1648984828 ("x86/efi: Update e820 with reserved EFI boot services...")
Cc: Taku Izumi <izumi.taku@jp.fujitsu.com>
Cc: Michael Weiser <michael@weiser.dinsnail.net>
Cc: Dave Young <dyoung@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/firmware/efi/fake_mem.c |   22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c
index bb9fc70d0cfa..6df51ba93ae8 100644
--- a/drivers/firmware/efi/fake_mem.c
+++ b/drivers/firmware/efi/fake_mem.c
@@ -67,13 +67,33 @@ void __init efi_fake_memmap(void)
 		return;
 	}
 
+	memset(new_memmap, 0, efi.memmap.desc_size * new_nr_map);
 	for (i = 0; i < nr_fake_mem; i++)
 		efi_memmap_insert(&efi.memmap, new_memmap, &efi_fake_mems[i]);
 
+	/*
+	 * efi_memmap_split_count() may over count the number of
+	 * required splits in the case when contiguous fake entries are
+	 * specified. Check that all new_nr_map entries were consumed.
+	 */
+	for (i = new_nr_map; i > 0; i--) {
+		efi_memory_desc_t *md;
+		u64 start, end;
+
+		md = new_memmap + efi.memmap.desc_size * (new_nr_map - i - 1);
+		end = md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) - 1;
+		start = md->phys_addr;
+
+		if (start == 0 && end + 1 == 0)
+			continue;
+		break;
+	}
+
 	/* swap into new EFI memmap */
 	early_memunmap(new_memmap, efi.memmap.desc_size * new_nr_map);
 
-	efi_memmap_install(new_memmap_phy, new_nr_map);
+	/* install only the valid entries */
+	efi_memmap_install(new_memmap_phy, i);
 
 	/* print new EFI memmap */
 	efi_print_memmap();


WARNING: multiple messages have this Message-ID (diff)
From: Dan Williams <dan.j.williams@intel.com>
To: mingo@redhat.com
Cc: Michael Weiser <michael@weiser.dinsnail.net>,
	linux-efi@vger.kernel.org,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	kexec@lists.infradead.org, linux-kernel@vger.kernel.org,
	Taku Izumi <izumi.taku@jp.fujitsu.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Dave Young <dyoung@redhat.com>, Ingo Molnar <mingo@kernel.org>
Subject: [PATCH] efi: Fix handling of multiple contiguous efi_fake_mem= entries
Date: Mon, 30 Dec 2019 11:58:23 -0800	[thread overview]
Message-ID: <157773590338.4153451.5898675419563883883.stgit@dwillia2-desk3.amr.corp.intel.com> (raw)

A recent test of efi_fake_mem=4G@9G:0x40000,4G@13G:0x40000 crashed with
a signature of:

    BUG: unable to handle page fault for address: ffffffffff281000
    [..]
    RIP: 0010:efi_memmap_insert+0x11d/0x191
    [..]
    Call Trace:
     ? bgrt_init+0xbe/0xbe
     ? efi_arch_mem_reserve+0x1cb/0x228
     ? acpi_parse_bgrt+0xa/0xd
     ? acpi_table_parse+0x86/0xb8
     ? acpi_boot_init+0x494/0x4e3
     ? acpi_parse_x2apic+0x87/0x87
     ? setup_acpi_sci+0xa2/0xa2
     ? setup_arch+0x8db/0x9e1
     ? start_kernel+0x6a/0x547
     ? secondary_startup_64+0xb6/0xc0

efi_memmap_insert() is attempting to insert entries past the end of the
new map. This condition is setup by efi_fake_mem() leaking empty entries
to the end of memory map, and then efi_arch_mem_reserve() trips over the
bad entry when attempting an additional efi_memmap_insert(). The empty
entry causes efi_memmap_insert() to attempt more memmap splits / copies
than efi_memmap_split_count() accounted for when sizing the new map.

Update efi_fake_memmap() to cleanup lagging empty entries.

This change is related to commit af1648984828 "x86/efi: Update e820 with
reserved EFI boot services data to fix kexec breakage" since that
introduces more occurrences where efi_memmap_insert() is invoked after
an efi_fake_mem= configuration has been parsed. Previously the side
effects of vestigial empty entries were benign, but with commit
af1648984828 that follow-on efi_memmap_insert() invocation triggers the
above crash signature.

Fixes: 0f96a99dab36 ("efi: Add 'efi_fake_mem' boot option")
Fixes: af1648984828 ("x86/efi: Update e820 with reserved EFI boot services...")
Cc: Taku Izumi <izumi.taku@jp.fujitsu.com>
Cc: Michael Weiser <michael@weiser.dinsnail.net>
Cc: Dave Young <dyoung@redhat.com>
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@kernel.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
 drivers/firmware/efi/fake_mem.c |   22 +++++++++++++++++++++-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/drivers/firmware/efi/fake_mem.c b/drivers/firmware/efi/fake_mem.c
index bb9fc70d0cfa..6df51ba93ae8 100644
--- a/drivers/firmware/efi/fake_mem.c
+++ b/drivers/firmware/efi/fake_mem.c
@@ -67,13 +67,33 @@ void __init efi_fake_memmap(void)
 		return;
 	}
 
+	memset(new_memmap, 0, efi.memmap.desc_size * new_nr_map);
 	for (i = 0; i < nr_fake_mem; i++)
 		efi_memmap_insert(&efi.memmap, new_memmap, &efi_fake_mems[i]);
 
+	/*
+	 * efi_memmap_split_count() may over count the number of
+	 * required splits in the case when contiguous fake entries are
+	 * specified. Check that all new_nr_map entries were consumed.
+	 */
+	for (i = new_nr_map; i > 0; i--) {
+		efi_memory_desc_t *md;
+		u64 start, end;
+
+		md = new_memmap + efi.memmap.desc_size * (new_nr_map - i - 1);
+		end = md->phys_addr + (md->num_pages << EFI_PAGE_SHIFT) - 1;
+		start = md->phys_addr;
+
+		if (start == 0 && end + 1 == 0)
+			continue;
+		break;
+	}
+
 	/* swap into new EFI memmap */
 	early_memunmap(new_memmap, efi.memmap.desc_size * new_nr_map);
 
-	efi_memmap_install(new_memmap_phy, new_nr_map);
+	/* install only the valid entries */
+	efi_memmap_install(new_memmap_phy, i);
 
 	/* print new EFI memmap */
 	efi_print_memmap();


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

             reply	other threads:[~2019-12-30 20:14 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-30 19:58 Dan Williams [this message]
2019-12-30 19:58 ` [PATCH] efi: Fix handling of multiple contiguous efi_fake_mem= entries Dan Williams
2019-12-31  1:46 ` Dave Young
2019-12-31  1:46   ` Dave Young
2019-12-31 21:11   ` Dan Williams
2019-12-31 21:11     ` Dan Williams
2019-12-31 22:21     ` Dan Williams
2019-12-31 22:21       ` Dan Williams
2020-01-01  3:23     ` Dave Young

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=157773590338.4153451.5898675419563883883.stgit@dwillia2-desk3.amr.corp.intel.com \
    --to=dan.j.williams@intel.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=dyoung@redhat.com \
    --cc=izumi.taku@jp.fujitsu.com \
    --cc=kexec@lists.infradead.org \
    --cc=linux-efi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=michael@weiser.dinsnail.net \
    --cc=mingo@kernel.org \
    --cc=mingo@redhat.com \
    --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 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.