linux-kernel - Re: [PATCH 2/2] x86/mm/pti: warn and stop when pti_clone

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]

Date:   Mon, 30 Nov 2020 08:37:15 -0800
From:   Dave Hansen <dave.hansen@...el.com>
To:     Lai Jiangshan <jiangshanlai@...il.com>,
        linux-kernel@...r.kernel.org
Cc:     Lai Jiangshan <laijs@...ux.alibaba.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>
Subject: Re: [PATCH 2/2] x86/mm/pti: warn and stop when pti_clone_pagetable()
 is on 1G page

On 11/30/20 7:25 AM, Lai Jiangshan wrote:
> --- a/arch/x86/mm/pti.c
> +++ b/arch/x86/mm/pti.c
> @@ -321,10 +321,10 @@ pti_clone_pgtable(unsigned long start, unsigned long end,
>  			break;
>  
>  		pgd = pgd_offset_k(addr);
> -		if (WARN_ON(pgd_none(*pgd)))
> +		if (WARN_ON(pgd_none(*pgd) || pgd_large(*pgd)))
>  			return;
>  		p4d = p4d_offset(pgd, addr);
> -		if (WARN_ON(p4d_none(*p4d)))
> +		if (WARN_ON(p4d_none(*p4d) || p4d_large(*p4d)))
>  			return;
>  
>  		pud = pud_offset(p4d, addr);
> @@ -333,6 +333,8 @@ pti_clone_pgtable(unsigned long start, unsigned long end,
>  			addr = round_up(addr + 1, PUD_SIZE);
>  			continue;
>  		}
> +		if (WARN_ON(pud_large(*pud)))
> +			return;

One bit of practical application missing from the changelog: Right now,
we only clone parts of the kernel image and the cpu entry area.  The cpu
entry area would be insane to map with 1G pages since it maps so many
different kinds of pages and has *small* mappings.

For the kernel image to have a 1GB area with uniform permissions seems
pretty far away to me.  It would be en even more remote possibility that
a large swath of it would need to be cloned for PTI.  Kernel text with a
non-PCID system is probably as close as we would get.  I'm also not even
sure we have the code to create 1GB mappings for parts of the image.

While I'm fine with this for robustness and self-documentation, I think
there needs to be a bit more on this in the changelog.

Also, wouldn't we be better off if we added warnings to the p*d_offset()
functions?  The real problem here, for instance, is passing a
pgd_large()==1 pgd to p4d_offset().