| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Tue, 8 Oct 2019 10:44:47 +0800
From: Baoquan He <bhe@...hat.com>
To: "Eric W. Biederman" <ebiederm@...ssion.com>
Cc: lijiang <lijiang@...hat.com>, Dave Young <dyoung@...hat.com>,
linux-kernel@...r.kernel.org, tglx@...utronix.de, mingo@...hat.com,
bp@...en8.de, hpa@...or.com, x86@...nel.org, jgross@...e.com,
dhowells@...hat.com, Thomas.Lendacky@....com, vgoyal@...hat.com,
kexec@...ts.infradead.org
Subject: Re: [PATCH v2] x86/kdump: Fix 'kmem -s' reported an invalid
freepointer when SME was active
On 10/07/19 at 12:12pm, Eric W. Biederman wrote:
> This has only been boot tested but I think this is about what we need.
>
> I feel like I haven't found and deleted all of the backup region code.
>
> I think it is important to have the reservation code in reseve_real_mode
> as the logic is fundamentally intertwined.
>
> Eric
>
>
> From: "Eric W. Biederman" <ebiederm@...ssion.com>
> Date: Mon, 7 Oct 2019 11:57:24 -0500
> Subject: [PATCH] x86/kexec: Always reserve the low 1MiB
>
> When the crashkernel kernel command line option is specified always
> reserve the low 1MiB. That way it does not need to be included
> in crash dumps or used for anything execept the processor trampolines
> that must live in the low 1MiB.
>
> The current handling of copying the low 1MiB runs into problems when
> SME is active. So just simplify everything and make it unnecessary
> to do anything with the low 1MiB.
>
> This comes at a cost of 640KiB. But when crash kernels need 32MiB or
> more to run this isn't much more, and it makes everything much more
> reliable.
>
> Signed-off-by: "Eric W. Biederman" <ebiederm@...ssion.com>
> ---
> arch/x86/include/asm/kexec.h | 4 ----
> arch/x86/kernel/crash.c | 19 -------------------
> arch/x86/purgatory/purgatory.c | 15 ---------------
> arch/x86/realmode/init.c | 10 ++++++++++
> 4 files changed, 10 insertions(+), 38 deletions(-)
>
> diff --git a/arch/x86/include/asm/kexec.h b/arch/x86/include/asm/kexec.h
> index 5e7d6b46de97..e36307ac324d 100644
> --- a/arch/x86/include/asm/kexec.h
> +++ b/arch/x86/include/asm/kexec.h
> @@ -66,10 +66,6 @@ struct kimage;
> # define KEXEC_ARCH KEXEC_ARCH_X86_64
> #endif
>
> -/* Memory to backup during crash kdump */
> -#define KEXEC_BACKUP_SRC_START (0UL)
> -#define KEXEC_BACKUP_SRC_END (640 * 1024UL - 1) /* 640K */
> -
> /*
> * This function is responsible for capturing register states if coming
> * via panic otherwise just fix up the ss and sp if coming via kernel
> diff --git a/arch/x86/kernel/crash.c b/arch/x86/kernel/crash.c
> index eb651fbde92a..dc4773d2f4a6 100644
> --- a/arch/x86/kernel/crash.c
> +++ b/arch/x86/kernel/crash.c
> @@ -409,31 +409,12 @@ int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
> return ret;
> }
>
> -static int determine_backup_region(struct resource *res, void *arg)
> -{
> - struct kimage *image = arg;
> -
> - image->arch.backup_src_start = res->start;
> - image->arch.backup_src_sz = resource_size(res);
> -
> - /* Expecting only one range for backup region */
> - return 1;
> -}
> -
> int crash_load_segments(struct kimage *image)
> {
> int ret;
> struct kexec_buf kbuf = { .image = image, .buf_min = 0,
> .buf_max = ULONG_MAX, .top_down = false };
>
> - /*
> - * Determine and load a segment for backup area. First 640K RAM
> - * region is backup source
> - */
> -
> - ret = walk_system_ram_res(KEXEC_BACKUP_SRC_START, KEXEC_BACKUP_SRC_END,
> - image, determine_backup_region);
> -
> /* Zero or postive return values are ok */
> if (ret < 0)
> return ret;
> diff --git a/arch/x86/purgatory/purgatory.c b/arch/x86/purgatory/purgatory.c
> index 3b95410ff0f8..448de04703ba 100644
> --- a/arch/x86/purgatory/purgatory.c
> +++ b/arch/x86/purgatory/purgatory.c
> @@ -22,20 +22,6 @@ u8 purgatory_sha256_digest[SHA256_DIGEST_SIZE] __section(.kexec-purgatory);
>
> struct kexec_sha_region purgatory_sha_regions[KEXEC_SEGMENT_MAX] __section(.kexec-purgatory);
>
> -/*
> - * On x86, second kernel requries first 640K of memory to boot. Copy
> - * first 640K to a backup region in reserved memory range so that second
> - * kernel can use first 640K.
> - */
> -static int copy_backup_region(void)
> -{
> - if (purgatory_backup_dest) {
> - memcpy((void *)purgatory_backup_dest,
> - (void *)purgatory_backup_src, purgatory_backup_sz);
> - }
> - return 0;
> -}
> -
> static int verify_sha256_digest(void)
> {
> struct kexec_sha_region *ptr, *end;
> @@ -66,7 +52,6 @@ void purgatory(void)
> for (;;)
> ;
> }
> - copy_backup_region();
> }
>
> /*
> diff --git a/arch/x86/realmode/init.c b/arch/x86/realmode/init.c
> index 7dce39c8c034..76c680ad23a1 100644
> --- a/arch/x86/realmode/init.c
> +++ b/arch/x86/realmode/init.c
> @@ -34,6 +34,16 @@ void __init reserve_real_mode(void)
>
> memblock_reserve(mem, size);
> set_real_mode_mem(mem);
> +
> +#ifdef CONFIG_KEXEC_CORE
> + /* When crashkernel is specified only use the low 1MiB for the
> + * real mode trampolines.
> + */
> + if (strstr(boot_command_line, "crashkernel=")) {
> + memblock_reserve(0, 1<<20);
> + pr_info("Reserving low 1MiB of memory for crashkernel\n");
> + }
Reserving low 1M looks good to me. The memblock reserved pages won't
enter into buddy allocator, unless they are freed explicitly with
memblock_free() later.
> +#endif /* CONFIG_KEXEC_CORE */
I doubt this patch can work in kdump kernel booting. Because the low 1MB
is not passed to kdump kernel as system RAM, please check below code.
/* Prepare memory map for crash dump kernel */
int crash_setup_memmap_entries(struct kimage *image, struct boot_params *params)
{
......
/* Add first 640K segment */
ei.addr = image->arch.backup_src_start;
ei.size = image->arch.backup_src_sz;
ei.type = E820_TYPE_RAM;
add_e820_entry(params, &ei);
......
}
You can see that image->arch.backup_src_start/backup_src_sz are zero.
Lianbo will take a test to check.
Thanks
Baoquan
Powered by blists - more mailing lists