linux-kernel - Re: Definition of PMD_FLAGS_DEC_WP in arch/x86/mm/mem_encrypt

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 9 Nov 2020 08:28:06 -0600
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Arvind Sankar <nivedita@...m.mit.edu>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Andy Lutomirski <luto@...nel.org>,
        Peter Zijlstra <peterz@...radead.org>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org
Subject: Re: Definition of PMD_FLAGS_DEC_WP in
 arch/x86/mm/mem_encrypt_identity.c

On 11/8/20 10:37 AM, Arvind Sankar wrote:
> Hi, I have a question about this definition in
> arch/x86/mm/mem_encrypt_identity.c:
> 
> 	#define PMD_FLAGS_LARGE         (__PAGE_KERNEL_LARGE_EXEC & ~_PAGE_GLOBAL)
> 
> 	#define PMD_FLAGS_DEC           PMD_FLAGS_LARGE
> 	#define PMD_FLAGS_DEC_WP        ((PMD_FLAGS_DEC & ~_PAGE_CACHE_MASK) | \
> 					 (_PAGE_PAT | _PAGE_PWT))
> 
> _PAGE_CACHE_MASK and _PAGE_PAT are for 4k pages, not 2M pages. The
> definition of PMD_FLAGS_DEC_WP clears the PSE bit by masking out
> _PAGE_CACHE_MASK, and sets it again by setting _PAGE_PAT, resulting in
> PMD_FLAGS_DEC_WP actually being write-through, not write-protected,
> using PAT index 1.
> 
> Shouldn't the definition be
> 
> 	#define PMD_FLAGS_DEC_WP	(PMD_FLAGS_DEC | _PAGE_PAT_LARGE | _PAGE_PWT)
> 
> for write-protected using PAT index 5?

Yes it should. There should probably be a _PAGE_CACHE_MASK_LARGE
definition so that the end result is:

#define PMD_FLAGS_DEC_WP	((PMD_FLAGS_DEC & ~_PAGE_CACHE_MASK_LARGE) | \
				(_PAGE_PAT_LARGE | _PAGE_PWT)

> 
> I guess the difference doesn't actually matter for encrypt-in-place? But
> mem_encrypt_boot.S takes pains to initialize PA5 to be write-protected,
> and it looks like it won't actually be used.

Given how early in the boot everything occurs and the cache flushing that
is performed related to the operations, it works. But this should be
fixed.

Are you planning on sending a patch?

Thanks,
Tom

> 
> Thanks.
>