Received-SPF: pass (google.com: domain of linux-kernel+bounces-169676-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Precedence: bulk
MIME-Version: 1.0
References: <20240409210254.660888920@goodmis.org> <20240409211351.075320273@goodmis.org>
 <202404091521.B63E85D@keescook> <20240409191156.5f92a15c@gandalf.local.home>
 <202404091638.2F98764A41@keescook> <Zhmgm86tzpanoweB@kernel.org>
 <20240412181940.3e1d99f7@gandalf.local.home> <202404151017.FC002AA5@keescook> <ZjJYV7ak5ApgNTBx@kernel.org>
In-Reply-To: <ZjJYV7ak5ApgNTBx@kernel.org>
From: Ard Biesheuvel <ardb@kernel.org>
Date: Mon, 6 May 2024 12:38:32 +0200
Message-ID: <CAMj1kXG3jiLahONhPkKD0VSngDnMQoUCkDmoCsWEzOHDZmhTiA@mail.gmail.com>
Subject: Re: [POC][RFC][PATCH 1/2] mm/x86: Add wildcard * option as memmap=nn*align:name
To: Mike Rapoport <rppt@kernel.org>
Cc: Kees Cook <keescook@chromium.org>, Steven Rostedt <rostedt@goodmis.org>, 
	linux-kernel@vger.kernel.org, linux-trace-kernel@vger.kernel.org, 
	Masami Hiramatsu <mhiramat@kernel.org>, Mark Rutland <mark.rutland@arm.com>, 
	Mathieu Desnoyers <mathieu.desnoyers@efficios.com>, Andrew Morton <akpm@linux-foundation.org>, 
	"Liam R. Howlett" <Liam.Howlett@oracle.com>, Vlastimil Babka <vbabka@suse.cz>, 
	Lorenzo Stoakes <lstoakes@gmail.com>, linux-mm@kvack.org, 
	Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, 
	Dave Hansen <dave.hansen@linux.intel.com>, x86@kernel.org, 
	"H. Peter Anvin" <hpa@zytor.com>, Peter Zijlstra <peterz@infradead.org>, Tony Luck <tony.luck@intel.com>, 
	"Guilherme G. Piccoli" <gpiccoli@igalia.com>, linux-hardening@vger.kernel.org, 
	Guenter Roeck <linux@roeck-us.net>, Ross Zwisler <zwisler@google.com>, wklin@google.com, 
	Vineeth Remanan Pillai <vineeth@bitbyteword.org>, Joel Fernandes <joel@joelfernandes.org>, 
	Suleiman Souhlal <suleiman@google.com>, Linus Torvalds <torvalds@linuxfoundation.org>, 
	Catalin Marinas <catalin.marinas@arm.com>, Will Deacon <will@kernel.org>
Content-Type: text/plain; charset="UTF-8"

On Wed, 1 May 2024 at 16:59, Mike Rapoport <rppt@kernel.org> wrote:
>
> On Mon, Apr 15, 2024 at 10:22:53AM -0700, Kees Cook wrote:
> > On Fri, Apr 12, 2024 at 06:19:40PM -0400, Steven Rostedt wrote:
> > > On Fri, 12 Apr 2024 23:59:07 +0300
> > > Mike Rapoport <rppt@kernel.org> wrote:
> > >
> > > > On Tue, Apr 09, 2024 at 04:41:24PM -0700, Kees Cook wrote:
> > > > > On Tue, Apr 09, 2024 at 07:11:56PM -0400, Steven Rostedt wrote:
> > > > > > On Tue, 9 Apr 2024 15:23:07 -0700
> > > > > > Kees Cook <keescook@chromium.org> wrote:
> > > > > >
> > > > > > > Do we need to involve e820 at all? I think it might be possible to just
> > > > > > > have pstore call request_mem_region() very early? Or does KASLR make
> > > > > > > that unstable?
> > > > > >
> > > > > > Yeah, would that give the same physical memory each boot, and can we
> > > > > > guarantee that KASLR will not map the kernel over the previous location?
> > > > >
> > > > > Hm, no, for physical memory it needs to get excluded very early, which
> > > > > means e820.
> > > >
> > > > Whatever memory is reserved in arch/x86/kernel/e820.c, that happens after
> > > > kaslr, so to begin with, a new memmap parameter should be also added to
> > > > parse_memmap in arch/x86/boot/compressed/kaslr.c to ensure the same
> > > > physical address will be available after KASLR.
> > >
> > > But doesn't KASLR only affect virtual memory not physical memory?
> >
> > KASLR for x86 (and other archs, like arm64) do both physical and virtual
> > base randomization.
> >
> > > This just makes sure the physical memory it finds will not be used by the
> > > system. Then ramoops does the mapping via vmap() I believe, to get a
> > > virtual address to access the physical address.
> >
> > I was assuming, since you were in the e820 code, that it was
> > manipulating that before KASLR chose a location. But if not, yeah, Mike
> > is right -- you need to make sure this is getting done before
> > decompress_kernel().
>
> Right now kaslr can handle up to 4 memmap regions and parse_memmap() in
> arch/x86/boot/compressed/kaslr.c should be updated for a new memmap type.
>
> But I think it's better to add a new kernel parameter as I suggested in
> another email and teach mem_avoid_memmap() in kaslr.c to deal with it, as
> well as with crashkernel=size@offset, btw.
>

The logic in arch/x86/boot/compressed/kaslr.c is now only used by non-EFI boot.

In general, I am highly skeptical that hopes and prayers are enough to
prevent the firmware from stepping on such a region, unless this is
only a best effort thing, and failures are acceptable. For instance,
booting an EFI system with/without an external display attached, or
with a USB device inserted (without even using it during boot) will
impact the memory map, to the extent that the E820 table derived from
it may look different. (EFI tries to keep the runtime regions in the
same place but the boot-time regions are allocated/freed on demand)

So I would strongly urge to address this properly, and work with
firmware folks to define some kind of protocol for this.