| 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
| ||
|
Message-ID: <ZmqK_zeGus_VxAHN@kernel.org>
Date: Thu, 13 Jun 2024 09:00:31 +0300
From: Mike Rapoport <rppt@...nel.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org,
Masami Hiramatsu <mhiramat@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Andrew Morton <akpm@...ux-foundation.org>,
"Liam R. Howlett" <Liam.Howlett@...cle.com>,
Vlastimil Babka <vbabka@...e.cz>,
Lorenzo Stoakes <lstoakes@...il.com>, linux-mm@...ck.org,
Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
"H. Peter Anvin" <hpa@...or.com>,
Peter Zijlstra <peterz@...radead.org>,
Kees Cook <keescook@...omium.org>, Tony Luck <tony.luck@...el.com>,
"Guilherme G. Piccoli" <gpiccoli@...lia.com>,
linux-hardening@...r.kernel.org, Guenter Roeck <linux@...ck-us.net>,
Ross Zwisler <zwisler@...gle.com>, wklin@...gle.com,
Vineeth Remanan Pillai <vineeth@...byteword.org>,
Joel Fernandes <joel@...lfernandes.org>,
Suleiman Souhlal <suleiman@...gle.com>,
Linus Torvalds <torvalds@...uxfoundation.org>,
Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>, Ard Biesheuvel <ardb@...nel.org>
Subject: Re: [PATCH v5 1/2] mm/memblock: Add "reserve_mem" to reserved named
memory at boot up
On Wed, Jun 12, 2024 at 08:34:36PM -0400, Steven Rostedt wrote:
> From: "Steven Rostedt (Google)" <rostedt@...dmis.org>
>
> In order to allow for requesting a memory region that can be used for
> things like pstore on multiple machines where the memory layout is not the
> same, add a new option to the kernel command line called "reserve_mem".
>
> The format is: reserve_mem=nn:align:name
>
> Where it will find nn amount of memory at the given alignment of align.
> The name field is to allow another subsystem to retrieve where the memory
> was found. For example:
>
> reserve_mem=12M:4096:oops ramoops.mem_name=oops
>
> Where ramoops.mem_name will tell ramoops that memory was reserved for it
> via the reserve_mem option and it can find it by calling:
>
> if (reserve_mem_find_by_name("oops", &start, &size)) {
> // start holds the start address and size holds the size given
>
> This is typically used for systems that do not wipe the RAM, and this
> command line will try to reserve the same physical memory on soft reboots.
> Note, it is not guaranteed to be the same location. For example, if KASLR
> places the kernel at the location of where the RAM reservation was from a
> previous boot, the new reservation will be at a different location. Any
> subsystem using this feature must add a way to verify that the contents of
> the physical memory is from a previous boot, as there may be cases where
> the memory will not be located at the same location.
>
> Not all systems may work either. There could be bit flips if the reboot
> goes through the BIOS. Using kexec to reboot the machine is likely to
> have better results in such cases.
>
> Link: https://lore.kernel.org/all/ZjJVnZUX3NZiGW6q@kernel.org/
>
> Suggested-by: Mike Rapoport <rppt@...nel.org>
> Tested-by: Guilherme G. Piccoli <gpiccoli@...lia.com>
> Signed-off-by: Steven Rostedt (Google) <rostedt@...dmis.org>
> ---
> Changes since v4: https://lore.kernel.org/linux-trace-kernel/20240611215801.443593152@goodmis.org
>
> - Add all checks about reserve_mem before allocation.
> This means reserved_mem_add() is now a void function.
>
> - Check for name duplications.
>
> - Fix compare of align to SMP_CACHE_BYTES ("<" instead of "<=")
>
> .../admin-guide/kernel-parameters.txt | 20 +++
> include/linux/mm.h | 2 +
> mm/memblock.c | 117 ++++++++++++++++++
> 3 files changed, 139 insertions(+)
>
> diff --git a/mm/memblock.c b/mm/memblock.c
> index d09136e040d3..739d106a9165 100644
> --- a/mm/memblock.c
> +++ b/mm/memblock.c
...
> +/*
> + * Parse reserve_mem=nn:align:name
> + */
> +static int __init reserve_mem(char *p)
> +{
> + phys_addr_t start, size, align;
> + char *name;
> + char *oldp;
> + int len;
> +
> + if (!p)
> + return -EINVAL;
> +
> + /* Check if there's room for more reserved memory */
> + if (reserved_mem_count >= RESERVE_MEM_MAX_ENTRIES)
> + return -EBUSY;
> +
> + oldp = p;
> + size = memparse(p, &p);
> + if (!size || p == oldp)
> + return -EINVAL;
> +
> + if (*p != ':')
> + return -EINVAL;
> +
> + align = memparse(p+1, &p);
> + if (*p != ':')
> + return -EINVAL;
> +
> + /*
> + * memblock_phys_alloc() doesn't like a zero size align,
> + * but it is OK for this command to have it.
> + */
> + if (align < SMP_CACHE_BYTES)
> + align = SMP_CACHE_BYTES;
> +
> + name = p + 1;
> + len = strlen(name);
> +
> + /* name needs to have length but not too big */
> + if (!len || len >= RESERVE_MEM_NAME_SIZE)
> + return -EINVAL;
> +
> + /* Make sure that name has text */
> + for (p = name; *p; p++) {
> + if (!isspace(*p))
> + break;
> + }
> + if (!*p)
> + return -EINVAL;
> +
> + /* Make sure the name is not already used (size is only updated if found) */
> + if (reserve_mem_find_by_name(name, &start, &size))
> + return -EBUSY;
I'd prefer another variable here rather than size. Will be more robust IMO.
> +
> + start = memblock_phys_alloc(size, align);
> + if (!start)
> + return -ENOMEM;
> +
> + reserved_mem_add(start, size, name);
> +
> + return 0;
> +}
> +__setup("reserve_mem=", reserve_mem);
> +
> #if defined(CONFIG_DEBUG_FS) && defined(CONFIG_ARCH_KEEP_MEMBLOCK)
> static const char * const flagname[] = {
> [ilog2(MEMBLOCK_HOTPLUG)] = "HOTPLUG",
> --
> 2.43.0
>
>
--
Sincerely yours,
Mike.
Powered by blists - more mailing lists