linux-kernel - Re: [PATCH v9 04/11] arm64: kexec_file: allocate memory walking through memblock list

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <648656ef-1f1e-b0ac-581c-aba1e62f4eee@arm.com>
Date:   Tue, 1 May 2018 18:46:09 +0100
From:   James Morse <james.morse@....com>
To:     AKASHI Takahiro <takahiro.akashi@...aro.org>
Cc:     catalin.marinas@....com, will.deacon@....com, dhowells@...hat.com,
        vgoyal@...hat.com, herbert@...dor.apana.org.au,
        davem@...emloft.net, dyoung@...hat.com, bhe@...hat.com,
        arnd@...db.de, ard.biesheuvel@...aro.org, bhsharma@...hat.com,
        kexec@...ts.infradead.org, linux-arm-kernel@...ts.infradead.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v9 04/11] arm64: kexec_file: allocate memory walking
 through memblock list

Hi Akashi,

On 25/04/18 07:26, AKASHI Takahiro wrote:
> We need to prevent firmware-reserved memory regions, particularly EFI
> memory map as well as ACPI tables, from being corrupted by loading
> kernel/initrd (or other kexec buffers). We also want to support memory
> allocation in top-down manner in addition to default bottom-up.
> So let's have arm64 specific arch_kexec_walk_mem() which will search
> for available memory ranges in usable memblock list,
> i.e. !NOMAP & !reserved, 

> instead of system resource tree.

Didn't we try to fix the system-resource-tree in order to fix regular-kexec to
be safe in the EFI-memory-map/ACPI-tables case?

It would be good to avoid having two ways of doing this, and I would like to
avoid having extra arch code...


> diff --git a/arch/arm64/kernel/machine_kexec_file.c b/arch/arm64/kernel/machine_kexec_file.c
> new file mode 100644
> index 000000000000..f9ebf54ca247
> --- /dev/null
> +++ b/arch/arm64/kernel/machine_kexec_file.c
> @@ -0,0 +1,57 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * kexec_file for arm64
> + *
> + * Copyright (C) 2018 Linaro Limited
> + * Author: AKASHI Takahiro <takahiro.akashi@...aro.org>
> + *

> + * Most code is derived from arm64 port of kexec-tools

How does kexec-tools walk memblock?


> + */
> +
> +#define pr_fmt(fmt) "kexec_file: " fmt
> +
> +#include <linux/ioport.h>
> +#include <linux/kernel.h>
> +#include <linux/kexec.h>
> +#include <linux/memblock.h>
> +
> +int arch_kexec_walk_mem(struct kexec_buf *kbuf,
> +				int (*func)(struct resource *, void *))
> +{
> +	phys_addr_t start, end;
> +	struct resource res;
> +	u64 i;
> +	int ret = 0;
> +
> +	if (kbuf->image->type == KEXEC_TYPE_CRASH)
> +		return func(&crashk_res, kbuf);
> +
> +	if (kbuf->top_down)
> +		for_each_mem_range_rev(i, &memblock.memory, &memblock.reserved,
> +				NUMA_NO_NODE, MEMBLOCK_NONE,
> +				&start, &end, NULL) {

for_each_free_mem_range_reverse() is a more readable version of this helper.

> +			if (!memblock_is_map_memory(start))
> +				continue;

Passing MEMBLOCK_NONE means this walk will never find MEMBLOCK_NOMAP memory.


> +			res.start = start;
> +			res.end = end;
> +			ret = func(&res, kbuf);
> +			if (ret)
> +				break;
> +		}
> +	else
> +		for_each_mem_range(i, &memblock.memory, &memblock.reserved,
> +				NUMA_NO_NODE, MEMBLOCK_NONE,
> +				&start, &end, NULL) {

for_each_free_mem_range()?

> +			if (!memblock_is_map_memory(start))
> +				continue;
> +
> +			res.start = start;
> +			res.end = end;
> +			ret = func(&res, kbuf);
> +			if (ret)
> +				break;
> +		}
> +
> +	return ret;
> +}
> 

With these changes, what we have is almost:
arch/powerpc/kernel/machine_kexec_file_64.c::arch_kexec_walk_mem() !
(the difference being powerpc doesn't yet support crash-kernels here)

If the argument is walking memblock gives a better answer than the stringy
walk_system_ram_res() thing, is there any mileage in moving this code into
kexec_file.c, and using it if !IS_ENABLED(CONFIG_ARCH_DISCARD_MEMBLOCK)?

This would save arm64/powerpc having near-identical implementations.
32bit arm keeps memblock if it has kexec, so it may be useful there too if
kexec_file_load() support is added.


Thanks,

James