Re: [PATCH] x86/boot/64: Make level2_kernel_pgt pages invalid outside kernel area.

From: Dave Hansen <dave.hansen@intel.com>
To: Steve Wahl <steve.wahl@hpe.com>
Cc: "Kirill A. Shutemov" <kirill@shutemov.name>,
	Dave Hansen <dave.hansen@linux.intel.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	"H. Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, Juergen Gross <jgross@suse.com>,
	Brijesh Singh <brijesh.singh@amd.com>,
	Jordan Borgner <mail@jordan-borgner.de>,
	Feng Tang <feng.tang@intel.com>,
	linux-kernel@vger.kernel.org, Baoquan He <bhe@redhat.com>,
	russ.anderson@hpe.com, dimitri.sivanich@hpe.com,
	mike.travis@hpe.com
Subject: Re: [PATCH] x86/boot/64: Make level2_kernel_pgt pages invalid outside kernel area.
Date: Mon, 16 Sep 2019 15:19:45 -0700	[thread overview]
Message-ID: <c8eb79bc-05ea-1658-06d6-58af86358840@intel.com> (raw)
In-Reply-To: <20190916171455.GJ7834@swahl-linux>

On 9/16/19 10:14 AM, Steve Wahl wrote:
> I'm intending to add something like this in the next version:
> 
> /*
>  * Only the region occupied by the kernel image has so far been checked against
>  * the table of usable memory regions provided by the firmware, so
>  * invalidate pages outside that region.  A page table entry that maps to
>  * a reserved area of memory would allow processor speculation into that
>  * area, and on some hardware (particularly the UV platform) speculation
>  * into reserved areas can cause a system halt.
>  */

This is a good start, but it's a bit imprecise.

The KASLR code ensures that the kernel image itself doesn't overlap a
reserved area, but it appears that we're mapping more than strictly just
the kernel image.

>> But, I'm left with lots of questions:
>>
>> Why do PMD-level changes fix this?  Is it because we 2MB pad the kernel
>> image?  Why can't we still get within 2MB of the memory address in
>> question?
> 
> This fix works for our hardware because the problematic reserved
> regions are 64M aligned, and going up to the next 2MB boundary from
> _end is not going to cross the next 64M boundary.

I'd really prefer that we fix this once and for all rather than kicking
the can down the road to the next hardware manufacturer that changes the
alignment.

> One could argue the next step would be going into
> boot/compressed/{kaslr.c, misc.c} and rounding the size of the kernel
> up to the next 2MB boundary to ensure the chosen random location is
> covered by usable RAM up to the next PMD-level boundary.  I did not go
> there because for us it is not necessary.

Doing this would *fully* fix this issue, right?  Seems like the right
thing to do to me.

>> If this is all about avoiding EFI reserved ranges, why doesn't the
>> patch *LOOK* At EFI reserved ranges?
> 
> Because the range the kernel image is located in is already checked
> against them in boot/compressed/kaslr.c.  This will now be explained
> in the comment I mention above, which you had not yet seen.

Ahh, thanks for the explanation.  This will make for good changelog
material.