On Sat, Nov 02, 2024 at 12:06:18PM +0100, Borislav Petkov wrote:
> Ok, I'll take your 2/2 next week and you can then send the cleanup ontop.
OMG what a mess this is. Please test the below before I apply it.
Then, when you do the cleanup, do the following:
- merge early_memremap_is_setup_data() with memremap_is_setup_data() into
a common __memremap_is_setup_data() and then add a bool early which
determines which memremap variant is called.
- unify the @size argument by dropping it and using a function local size.
What we have there now is the definition of bitrot. :-\
- replace all sizeof(*data), sizeof(struct setup_data) with a macro definition
above the functions to unify it properly.
What an ugly mess... :-\
---
From: Baoquan He <bhe@xxxxxxxxxx>
Date: Wed, 11 Sep 2024 16:16:15 +0800
Subject: [PATCH] x86/mm: Fix a kdump kernel failure on SME system when
CONFIG_IMA_KEXEC=y
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
The kdump kernel is broken on SME systems with CONFIG_IMA_KEXEC=y enabled.
Debugging traced the issue back to
b69a2afd5afc ("x86/kexec: Carry forward IMA measurement log on kexec").
Testing was previously not conducted on SME systems with CONFIG_IMA_KEXEC
enabled, which led to the oversight, with the following incarnation:
...
ima: No TPM chip found, activating TPM-bypass!
Loading compiled-in module X.509 certificates
Loaded X.509 cert 'Build time autogenerated kernel key: 18ae0bc7e79b64700122bb1d6a904b070fef2656'
ima: Allocated hash algorithm: sha256
Oops: general protection fault, probably for non-canonical address 0xcfacfdfe6660003e: 0000 [#1] PREEMPT SMP NOPTI
CPU: 0 UID: 0 PID: 1 Comm: swapper/0 Not tainted 6.11.0-rc2+ #14
Hardware name: Dell Inc. PowerEdge R7425/02MJ3T, BIOS 1.20.0 05/03/2023
RIP: 0010:ima_restore_measurement_list
Call Trace:
<TASK>
? show_trace_log_lvl
? show_trace_log_lvl
? ima_load_kexec_buffer
? __die_body.cold
? die_addr
? exc_general_protection
? asm_exc_general_protection
? ima_restore_measurement_list
? vprintk_emit
? ima_load_kexec_buffer
ima_load_kexec_buffer
ima_init
? __pfx_init_ima
init_ima
? __pfx_init_ima
do_one_initcall
do_initcalls
? __pfx_kernel_init
kernel_init_freeable
kernel_init
ret_from_fork
? __pfx_kernel_init
ret_from_fork_asm
</TASK>
Modules linked in:
---[ end trace 0000000000000000 ]---
...
Kernel panic - not syncing: Fatal exception
Kernel Offset: disabled
Rebooting in 10 seconds..
Adding debug printks showed that the stored addr and size of ima_kexec buffer
are not decrypted correctly like:
ima: ima_load_kexec_buffer, buffer:0xcfacfdfe6660003e, size:0xe48066052d5df359
Three types of setup_data info
— SETUP_EFI,
- SETUP_IMA, and
- SETUP_RNG_SEED
are passed to the kexec/kdump kernel. Only the ima_kexec buffer
experienced incorrect decryption. Debugging identified a bug in
early_memremap_is_setup_data(), where an incorrect range calculation
occurred due to the len variable in struct setup_data ended up only
representing the length of the data field, excluding the struct's size,
and thus leading to miscalculation.
Address a similar issue in memremap_is_setup_data() while at it.
[ bp: Heavily massage. ]
Fixes: b3c72fc9a78e ("x86/boot: Introduce setup_indirect")
Signed-off-by: Baoquan He <bhe@xxxxxxxxxx>
Signed-off-by: Borislav Petkov (AMD) <bp@xxxxxxxxx>
Acked-by: Tom Lendacky <thomas.lendacky@xxxxxxx>
Cc: <stable@xxxxxxxxxx>
Link: https://lore.kernel.org/r/20240911081615.262202-3-bhe@xxxxxxxxxx
---
arch/x86/mm/ioremap.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 70b02fc61d93..8d29163568a7 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -656,7 +656,8 @@ static bool memremap_is_setup_data(resource_size_t phys_addr,
paddr_next = data->next;
len = data->len;
- if ((phys_addr > paddr) && (phys_addr < (paddr + len))) {
+ if ((phys_addr > paddr) &&
+ (phys_addr < (paddr + sizeof(struct setup_data) + len))) {
memunmap(data);
return true;
}
@@ -718,7 +719,8 @@ static bool __init early_memremap_is_setup_data(resource_size_t phys_addr,
paddr_next = data->next;
len = data->len;
- if ((phys_addr > paddr) && (phys_addr < (paddr + len))) {
+ if ((phys_addr > paddr) &&
+ (phys_addr < (paddr + sizeof(struct setup_data) + len))) {
early_memunmap(data, sizeof(*data));
return true;
}
--
2.43.0
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
_______________________________________________
kexec mailing list
kexec@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/kexec
