FAILED: patch "[PATCH] efi/x86: Free EFI memory map only when installing a new one." failed to apply to 5.10-stable tree

PATCH In review [email protected] 3 participants 7 messages Jun 24, 2024 – Jul 2, 2024

Subsystems (3)

Stable branch x86 EFI

lore

Share & tools

lore

[email protected]

author maintainer

2024-06-24 16:42 UTC #0 lore raw

The patch below does not apply to the 5.10-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-5.10.y
git checkout FETCH_HEAD
git cherry-pick -x 75dde792d6f6c2d0af50278bd374bf0c512fe196
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2024062455-glazing-flask-cf0c@gregkh' --subject-prefix 'PATCH 5.10.y' HEAD^..

Possible dependencies:

75dde792d6f6 ("efi/x86: Free EFI memory map only when installing a new one.")
d85e3e349407 ("efi: xen: Set EFI_PARAVIRT for Xen dom0 boot on all architectures") fdc6d38d64a2 ("efi: memmap: Move manipulation routines into x86 arch tree")
1df4d1724baa ("drivers: fix typo in firmware/efi/memmap.c")
db01ea882bf6 ("efi: Correct comment on efi_memmap_alloc")
3ecc68349bba ("memblock: rename memblock_free to memblock_phys_free")
fa27717110ae ("memblock: drop memblock_free_early_nid() and memblock_free_early()") 658aafc8139c ("memblock: exclude MEMBLOCK_NOMAP regions from kmemleak")
c6460daea23d ("Merge tag 'for-linus-5.15b-rc2-tag' of git://git.kernel.org/pub/scm/linux/kernel/git/xen/tip")

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 75dde792d6f6c2d0af50278bd374bf0c512fe196 Mon Sep 17 00:00:00 2001
From: Ard Biesheuvel <[email protected]>
Date: Mon, 10 Jun 2024 16:02:13 +0200
Subject: [PATCH] efi/x86: Free EFI memory map only when installing a new one.

The logic in __efi_memmap_init() is shared between two different execution flows:
- mapping the EFI memory map early or late into the kernel VA space, so that its entries can be accessed;
- the x86 specific cloning of the EFI memory map in order to insert new entries that are created as a result of making a memory reservation via a call to efi_mem_reserve().

In the former case, the underlying memory containing the kernel's view of the EFI memory map (which may be heavily modified by the kernel itself on x86) is not modified at all, and the only thing that changes is the virtual mapping of this memory, which is different between early and late boot.

In the latter case, an entirely new allocation is created that carries a new, updated version of the kernel's view of the EFI memory map. When installing this new version, the old version will no longer be referenced, and if the memory was allocated by the kernel, it will leak unless it gets freed.

The logic that implements this freeing currently lives on the code path that is shared between these two use cases, but it should only apply to the latter. So move it to the correct spot.

While at it, drop the dummy definition for non-x86 architectures, as that is no longer needed.

Cc <[email protected]>

Fixes f0ef6523475f ("efi: Fix efi_memmap_alloc() leaks")

Tested-by Ashish Kalra <[email protected]>

Link https://lore.kernel.org/all/[email protected] open here

Signed-off-by Ard Biesheuvel <[email protected]>

arch/x86/include/asm/efi.h +0 -1

index 1dc600fa3ba5..481096177500 100644
@@ -401,7 +401,6 @@ extern int __init efi_memmap_alloc(unsigned int num_entries,
 				   struct efi_memory_map_data *data);
 extern void __efi_memmap_free(u64 phys, unsigned long size,
 			      unsigned long flags);
-#define __efi_memmap_free __efi_memmap_free
 
 extern int __init efi_memmap_install(struct efi_memory_map_data *data);
 extern int __init efi_memmap_split_count(efi_memory_desc_t *md,

arch/x86/platform/efi/memmap.c +11 -1

index 4ef20b49eb5e..6ed1935504b9 100644
@@ -92,12 +92,22 @@ int __init efi_memmap_alloc(unsigned int num_entries,
  */
 int __init efi_memmap_install(struct efi_memory_map_data *data)
 {
+	unsigned long size = efi.memmap.desc_size * efi.memmap.nr_map;
+	unsigned long flags = efi.memmap.flags;
+	u64 phys = efi.memmap.phys_map;
+	int ret;
+
 	efi_memmap_unmap();
 
 	if (efi_enabled(EFI_PARAVIRT))
 		return 0;
 
-	return __efi_memmap_init(data);
+	ret = __efi_memmap_init(data);
+	if (ret)
+		return ret;
+
+	__efi_memmap_free(phys, size, flags);
+	return 0;
 }
 
 /**

drivers/firmware/efi/memmap.c +0 -9

index 3365944f7965..34109fd86c55 100644
@@ -15,10 +15,6 @@
 #include <asm/early_ioremap.h>
 #include <asm/efi.h>
 
-#ifndef __efi_memmap_free
-#define __efi_memmap_free(phys, size, flags) do { } while (0)
-#endif
-
 /**
  * __efi_memmap_init - Common code for mapping the EFI memory map
  * @data: EFI memory map data

@@ -51,11 +47,6 @@ int __init __efi_memmap_init(struct efi_memory_map_data *data)
 		return -ENOMEM;
 	}
 
-	if (efi.memmap.flags & (EFI_MEMMAP_MEMBLOCK | EFI_MEMMAP_SLAB))
-		__efi_memmap_free(efi.memmap.phys_map,
-				  efi.memmap.desc_size * efi.memmap.nr_map,
-				  efi.memmap.flags);
-
 	map.phys_map = data->phys_map;
 	map.nr_map = data->size / data->desc_size;
 	map.map_end = map.map + data->size;

Ard Biesheuvel

2024-06-29 15:14 UTC #1 lore raw

PATCH 1/5 drivers: fix typo in firmware/efi/memmap.c

From: Zheng Zhi Yuan <[email protected]>

[ Commit 1df4d1724baafa55e9803414ebcdf1ca702bc958 upstream ]

This patch fixes the spelling error in firmware/efi/memmap.c, changing it to the correct word.

Signed-off-by Zheng Zhi Yuan <[email protected]>

Signed-off-by Ard Biesheuvel <[email protected]>

---
 drivers/firmware/efi/memmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/firmware/efi/memmap.c +1 -1

index 2ff1883dc788..b4070b5e4c45 100644
@@ -245,7 +245,7 @@ int __init efi_memmap_install(struct efi_memory_map_data *data)
  * @range: Address range (start, end) to split around
  *
  * Returns the number of additional EFI memmap entries required to
- * accomodate @range.
+ * accommodate @range.
  */
 int __init efi_memmap_split_count(efi_memory_desc_t *md, struct range *range)
 {

Signature · 1 line

2.45.2.803.g4e1b14247a-goog

Ard Biesheuvel

2024-06-29 15:14 UTC #2 lore raw

PATCH 2/5 efi: Correct comment on efi_memmap_alloc

From: Liu Zixian <[email protected]>

[ Commit db01ea882bf601252dad57242655da17fd9ad2f5 upstream ]

Returning zero means success now.

Signed-off-by Liu Zixian <[email protected]>

Signed-off-by Ard Biesheuvel <[email protected]>

---
 drivers/firmware/efi/memmap.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

drivers/firmware/efi/memmap.c +1 -2

index b4070b5e4c45..3aaf717aad05 100644
@@ -59,8 +59,7 @@ static void __init efi_memmap_free(void)
  * Depending on whether mm_init() has already been invoked or not,
  * either memblock or "normal" page allocation is used.
  *
- * Returns the physical address of the allocated memory map on
- * success, zero on failure.
+ * Returns zero on success, a negative error code on failure.
  */
 int __init efi_memmap_alloc(unsigned int num_entries,
 		struct efi_memory_map_data *data)

Signature · 1 line

2.45.2.803.g4e1b14247a-goog

Ard Biesheuvel

2024-06-29 15:14 UTC #3 lore raw

PATCH 3/5 efi: memmap: Move manipulation routines into x86 arch tree

In reply to Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

[ Commit fdc6d38d64a20c542b1867ebeb8dd03b98829336 upstream ]

The EFI memory map is a description of the memory layout as provided by the firmware, and only x86 manipulates it in various different ways for its own memory bookkeeping. So let's move the memmap routines that are only used by x86 into the x86 arch tree.

[ardb: minor tweaks for linux-5.10.y backport]

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/x86/include/asm/efi.h     |  12 ++
 arch/x86/platform/efi/Makefile |   3 +-
 arch/x86/platform/efi/memmap.c | 236 ++++++++++++++++++++++++++++++++
 drivers/firmware/efi/memmap.c  | 240 ++-------------------------------
 include/linux/efi.h            |  10 +-
 5 files changed, 261 insertions(+), 240 deletions(-)
 create mode 100644 arch/x86/platform/efi/memmap.c

arch/x86/include/asm/efi.h +12 -0

index bc9758ef292e..01c0410ae97a 100644
@@ -382,4 +382,16 @@ static inline void efi_fake_memmap_early(void)
 }
 #endif
 
+extern int __init efi_memmap_alloc(unsigned int num_entries,
+				   struct efi_memory_map_data *data);
+extern void __efi_memmap_free(u64 phys, unsigned long size,
+			      unsigned long flags);
+#define __efi_memmap_free __efi_memmap_free
+
+extern int __init efi_memmap_install(struct efi_memory_map_data *data);
+extern int __init efi_memmap_split_count(efi_memory_desc_t *md,
+					 struct range *range);
+extern void __init efi_memmap_insert(struct efi_memory_map *old_memmap,
+				     void *buf, struct efi_mem_range *mem);
+
 #endif /* _ASM_X86_EFI_H */

arch/x86/platform/efi/Makefile +2 -1

index 84b09c230cbd..196f61fdbf28 100644
@@ -3,5 +3,6 @@ OBJECT_FILES_NON_STANDARD_efi_thunk_$(BITS).o := y
 KASAN_SANITIZE := n
 GCOV_PROFILE := n
 
-obj-$(CONFIG_EFI) 		+= quirks.o efi.o efi_$(BITS).o efi_stub_$(BITS).o
+obj-$(CONFIG_EFI) 		+= memmap.o quirks.o efi.o efi_$(BITS).o \
+				   efi_stub_$(BITS).o
 obj-$(CONFIG_EFI_MIXED)		+= efi_thunk_$(BITS).o

arch/x86/platform/efi/memmap.c new file +236 -0

236 lines not on this page · raw message

drivers/firmware/efi/memmap.c +10 ~~-230~~

265 lines not on this page · raw message

include/linux/efi.h +1 -9

index 084990390420..2fd321da5c4b 100644
@@ -633,18 +633,10 @@ static inline efi_status_t efi_query_variable_store(u32 attributes,
 #endif
 extern void __iomem *efi_lookup_mapped_addr(u64 phys_addr);
 
-extern int __init efi_memmap_alloc(unsigned int num_entries,
-				   struct efi_memory_map_data *data);
-extern void __efi_memmap_free(u64 phys, unsigned long size,
-			      unsigned long flags);
+extern int __init __efi_memmap_init(struct efi_memory_map_data *data);
 extern int __init efi_memmap_init_early(struct efi_memory_map_data *data);
 extern int __init efi_memmap_init_late(phys_addr_t addr, unsigned long size);
 extern void __init efi_memmap_unmap(void);
-extern int __init efi_memmap_install(struct efi_memory_map_data *data);
-extern int __init efi_memmap_split_count(efi_memory_desc_t *md,
-					 struct range *range);
-extern void __init efi_memmap_insert(struct efi_memory_map *old_memmap,
-				     void *buf, struct efi_mem_range *mem);
 
 #ifdef CONFIG_EFI_ESRT
 extern void __init efi_esrt_init(void);

Signature · 1 line

2.45.2.803.g4e1b14247a-goog

Ard Biesheuvel

2024-06-29 15:14 UTC #4 lore raw

PATCH 4/5 efi: xen: Set EFI_PARAVIRT for Xen dom0 boot on all architectures

In reply to Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

[ Commit d85e3e34940788578eeffd94e8b7e1d28e7278e9 upstream ]

Currently, the EFI_PARAVIRT flag is only used by Xen dom0 boot on x86, even though other architectures also support pseudo-EFI boot, where the core kernel is invoked directly and provided with a set of data tables that resemble the ones constructed by the EFI stub, which never actually runs in that case.

Let's fix this inconsistency, and always set this flag when booting dom0 via the EFI boot path. Note that Xen on x86 does not provide the EFI memory map in this case, whereas other architectures do, so move the associated EFI_PARAVIRT check into the x86 platform code.

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/x86/platform/efi/efi.c      | 8 +++++---
 arch/x86/platform/efi/memmap.c   | 3 +++
 drivers/firmware/efi/fdtparams.c | 4 ++++
 drivers/firmware/efi/memmap.c    | 3 ---
 4 files changed, 12 insertions(+), 6 deletions(-)

arch/x86/platform/efi/efi.c +5 -3

index 8a26e705cb06..41229bcbe0d9 100644
@@ -234,9 +234,11 @@ int __init efi_memblock_x86_reserve_range(void)
 	data.desc_size		= e->efi_memdesc_size;
 	data.desc_version	= e->efi_memdesc_version;
 
-	rv = efi_memmap_init_early(&data);
-	if (rv)
-		return rv;
+	if (!efi_enabled(EFI_PARAVIRT)) {
+		rv = efi_memmap_init_early(&data);
+		if (rv)
+			return rv;
+	}
 
 	if (add_efi_memmap || do_efi_soft_reserve())
 		do_add_efi_memmap();

arch/x86/platform/efi/memmap.c +3 -0

index 620af26b55c0..241464b6dd03 100644
@@ -94,6 +94,9 @@ int __init efi_memmap_install(struct efi_memory_map_data *data)
 {
 	efi_memmap_unmap();
 
+	if (efi_enabled(EFI_PARAVIRT))
+		return 0;
+
 	return __efi_memmap_init(data);
 }

drivers/firmware/efi/fdtparams.c +4 -0

index e901f8564ca0..0ec83ba58097 100644
@@ -30,11 +30,13 @@ static __initconst const char name[][22] = {
 
 static __initconst const struct {
 	const char	path[17];
+	u8		paravirt;
 	const char	params[PARAMCOUNT][26];
 } dt_params[] = {
 	{
 #ifdef CONFIG_XEN    //  <-------17------>
 		.path = "/hypervisor/uefi",
+		.paravirt = 1,
 		.params = {
 			[SYSTAB] = "xen,uefi-system-table",
 			[MMBASE] = "xen,uefi-mmap-start",

@@ -121,6 +123,8 @@ u64 __init efi_get_fdt_params(struct efi_memory_map_data *mm)
 			pr_err("Can't find property '%s' in DT!\n", pname);
 			return 0;
 		}
+		if (dt_params[i].paravirt)
+			set_bit(EFI_PARAVIRT, &efi.flags);
 		return systab;
 	}
 notfound:

drivers/firmware/efi/memmap.c +0 -3

index e6256c48284e..a1180461a445 100644
@@ -39,9 +39,6 @@ int __init __efi_memmap_init(struct efi_memory_map_data *data)
 	struct efi_memory_map map;
 	phys_addr_t phys_map;
 
-	if (efi_enabled(EFI_PARAVIRT))
-		return 0;
-
 	phys_map = data->phys_map;
 
 	if (data->flags & EFI_MEMMAP_LATE)

Signature · 1 line

2.45.2.803.g4e1b14247a-goog

Ard Biesheuvel

2024-06-29 15:14 UTC #5 lore raw

PATCH 5/5 efi/x86: Free EFI memory map only when installing a new one.

In reply to Ard Biesheuvel

From: Ard Biesheuvel <[email protected]>

[ Commit 75dde792d6f6c2d0af50278bd374bf0c512fe196 upstream ]

The logic that implements this freeing currently lives on the code path that is shared between these two use cases, but it should only apply to the latter. So move it to the correct spot.

While at it, drop the dummy definition for non-x86 architectures, as that is no longer needed.

Cc <[email protected]>

Fixes f0ef6523475f ("efi: Fix efi_memmap_alloc() leaks")

Tested-by Ashish Kalra <[email protected]>

Link https://lore.kernel.org/all/[email protected] open here

Signed-off-by Ard Biesheuvel <[email protected]>

---
 arch/x86/include/asm/efi.h     |  1 -
 arch/x86/platform/efi/memmap.c | 12 +++++++++++-
 drivers/firmware/efi/memmap.c  |  9 ---------
 3 files changed, 11 insertions(+), 11 deletions(-)

arch/x86/include/asm/efi.h +0 -1

index 01c0410ae97a..7c675832712d 100644
@@ -386,7 +386,6 @@ extern int __init efi_memmap_alloc(unsigned int num_entries,
 				   struct efi_memory_map_data *data);
 extern void __efi_memmap_free(u64 phys, unsigned long size,
 			      unsigned long flags);
-#define __efi_memmap_free __efi_memmap_free
 
 extern int __init efi_memmap_install(struct efi_memory_map_data *data);
 extern int __init efi_memmap_split_count(efi_memory_desc_t *md,

arch/x86/platform/efi/memmap.c +11 -1

index 241464b6dd03..872d310c426e 100644
@@ -92,12 +92,22 @@ int __init efi_memmap_alloc(unsigned int num_entries,
  */
 int __init efi_memmap_install(struct efi_memory_map_data *data)
 {
+	unsigned long size = efi.memmap.desc_size * efi.memmap.nr_map;
+	unsigned long flags = efi.memmap.flags;
+	u64 phys = efi.memmap.phys_map;
+	int ret;
+
 	efi_memmap_unmap();
 
 	if (efi_enabled(EFI_PARAVIRT))
 		return 0;
 
-	return __efi_memmap_init(data);
+	ret = __efi_memmap_init(data);
+	if (ret)
+		return ret;
+
+	__efi_memmap_free(phys, size, flags);
+	return 0;
 }
 
 /**

drivers/firmware/efi/memmap.c +0 -9

index a1180461a445..77dd20f9df31 100644
@@ -15,10 +15,6 @@
 #include <asm/early_ioremap.h>
 #include <asm/efi.h>
 
-#ifndef __efi_memmap_free
-#define __efi_memmap_free(phys, size, flags) do { } while (0)
-#endif
-
 /**
  * __efi_memmap_init - Common code for mapping the EFI memory map
  * @data: EFI memory map data

@@ -51,11 +47,6 @@ int __init __efi_memmap_init(struct efi_memory_map_data *data)
 		return -ENOMEM;
 	}
 
-	if (efi.memmap.flags & (EFI_MEMMAP_MEMBLOCK | EFI_MEMMAP_SLAB))
-		__efi_memmap_free(efi.memmap.phys_map,
-				  efi.memmap.desc_size * efi.memmap.nr_map,
-				  efi.memmap.flags);
-
 	map.phys_map = data->phys_map;
 	map.nr_map = data->size / data->desc_size;
 	map.map_end = map.map + data->size;

Signature · 1 line

2.45.2.803.g4e1b14247a-goog

Greg KH

2024-07-02 10:35 UTC #6 lore raw

Re: [PATCH 5.10.y 1/5] drivers: fix typo in firmware/efi/memmap.c

In reply to Ard Biesheuvel

Quoted 30 lines · Ard Biesheuvel

From: Zheng Zhi Yuan <[email protected]>
[ Commit 1df4d1724baafa55e9803414ebcdf1ca702bc958 upstream ]
This patch fixes the spelling error in firmware/efi/memmap.c, changing it to the correct word.
Signed-off-by Zheng Zhi Yuan <[email protected]>
Signed-off-by Ard Biesheuvel <[email protected]>
---
 drivers/firmware/efi/memmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/firmware/efi/memmap.c +1 -1
index 2ff1883dc788..b4070b5e4c45 100644
@@ -245,7 +245,7 @@ int __init efi_memmap_install(struct efi_memory_map_data *data)
  * @range: Address range (start, end) to split around
  *
  * Returns the number of additional EFI memmap entries required to
- * accomodate @range.
+ * accommodate @range.
  */
 int __init efi_memmap_split_count(efi_memory_desc_t *md, struct range *range)
 {
Signature · 1 line
2.45.2.803.g4e1b14247a-goog

All now queued up, thanks.

greg k-h