linux-kernel - Re: [PATCH v3 2/2] x86/mm/ident_map: Use gbpages only where full GB page should be mapped.

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: <ZqEU7gC9VU_loVfV@swahl-home.5wahls.com>
Date: Wed, 24 Jul 2024 09:51:26 -0500
From: Steve Wahl <steve.wahl@....com>
To: Steve Wahl <steve.wahl@....com>, Dave Hansen <dave.hansen@...ux.intel.com>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
        Borislav Petkov <bp@...en8.de>, x86@...nel.org,
        "H. Peter Anvin" <hpa@...or.com>, linux-kernel@...r.kernel.org,
        Pavin Joseph <me@...injoseph.com>, Eric Hagberg <ehagberg@...il.com>
Cc: Simon Horman <horms@...ge.net.au>, Eric Biederman <ebiederm@...ssion.com>,
        Dave Young <dyoung@...hat.com>, Sarah Brofeldt <srhb@....dk>,
        Russ Anderson <rja@....com>, Dimitri Sivanich <sivanich@....com>,
        Hou Wenlong <houwenlong.hwl@...group.com>,
        Andrew Morton <akpm@...ux-foundation.org>, Baoquan He <bhe@...hat.com>,
        Yuntao Wang <ytcoode@...il.com>, Bjorn Helgaas <bhelgaas@...gle.com>,
        Joerg Roedel <jroedel@...e.de>, Michael Roth <michael.roth@....com>,
        Tao Liu <ltao@...hat.com>, kexec@...ts.infradead.org,
        "Kalra, Ashish" <ashish.kalra@....com>,
        Ard Biesheuvel <ardb@...nel.org>, linux-efi@...r.kernel.org
Subject: Re: [PATCH v3 2/2] x86/mm/ident_map: Use gbpages only where full GB
 page should be mapped.

Gentle ping: Can someone please take the time to review this patch?

This patch was previously approved by Dave Hansen, but was reverted.

Full series (of 2 + cover leter) viewable at:

https://lore.kernel.org/all/20240717213121.3064030-1-steve.wahl@hpe.com/

Thanks.

--> Steve Wahl, HPE

On Wed, Jul 17, 2024 at 04:31:21PM -0500, Steve Wahl wrote:
> When ident_pud_init() uses only gbpages to create identity maps, large
> ranges of addresses not actually requested can be included in the
> resulting table; a 4K request will map a full GB.  This can include a
> lot of extra address space past that requested, including areas marked
> reserved by the BIOS.  That allows processor speculation into reserved
> regions, that on UV systems can cause system halts.
> 
> Only use gbpages when map creation requests include the full GB page
> of space.  Fall back to using smaller 2M pages when only portions of a
> GB page are included in the request.
> 
> No attempt is made to coalesce mapping requests. If a request requires
> a map entry at the 2M (pmd) level, subsequent mapping requests within
> the same 1G region will also be at the pmd level, even if adjacent or
> overlapping such requests could have been combined to map a full
> gbpage.  Existing usage starts with larger regions and then adds
> smaller regions, so this should not have any great consequence.
> 
> Signed-off-by: Steve Wahl <steve.wahl@....com>
> Tested-by: Pavin Joseph <me@...injoseph.com>
> Tested-by: Sarah Brofeldt <srhb@....dk>
> Tested-by: Eric Hagberg <ehagberg@...il.com>
> ---
>  arch/x86/mm/ident_map.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/mm/ident_map.c b/arch/x86/mm/ident_map.c
> index 968d7005f4a7..a204a332c71f 100644
> --- a/arch/x86/mm/ident_map.c
> +++ b/arch/x86/mm/ident_map.c
> @@ -26,18 +26,31 @@ static int ident_pud_init(struct x86_mapping_info *info, pud_t *pud_page,
>  	for (; addr < end; addr = next) {
>  		pud_t *pud = pud_page + pud_index(addr);
>  		pmd_t *pmd;
> +		bool use_gbpage;
>  
>  		next = (addr & PUD_MASK) + PUD_SIZE;
>  		if (next > end)
>  			next = end;
>  
> -		if (info->direct_gbpages) {
> -			pud_t pudval;
> +		/* if this is already a gbpage, this portion is already mapped */
> +		if (pud_leaf(*pud))
> +			continue;
> +
> +		/* Is using a gbpage allowed? */
> +		use_gbpage = info->direct_gbpages;
>  
> -			if (pud_present(*pud))
> -				continue;
> +		/* Don't use gbpage if it maps more than the requested region. */
> +		/* at the begining: */
> +		use_gbpage &= ((addr & ~PUD_MASK) == 0);
> +		/* ... or at the end: */
> +		use_gbpage &= ((next & ~PUD_MASK) == 0);
> +
> +		/* Never overwrite existing mappings */
> +		use_gbpage &= !pud_present(*pud);
> +
> +		if (use_gbpage) {
> +			pud_t pudval;
>  
> -			addr &= PUD_MASK;
>  			pudval = __pud((addr - info->offset) | info->page_flag);
>  			set_pud(pud, pudval);
>  			continue;
> -- 
> 2.26.2
> 

-- 
Steve Wahl, Hewlett Packard Enterprise