[PATCH v1 0/2] x86: Secure Memory Encryption (SME) fixes 2017-07-26

[PATCH v1 0/2] x86: Secure Memory Encryption (SME) fixes 2017-07-26

[Tom Lendacky]

This patch series addresses some issues found during further testing of
Secure Memory Encryption (SME).

The following fixes are included in this update series:

- Fix a cache-related memory corruption when kexec is invoked in
  successive instances
- Remove the encryption mask from the protection properties returned
  by arch_apei_get_mem_attribute() when SME is active

---

This patch series is based off of the master branch of tip:
  https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git master

  Commit 8333bcad393c ("Merge branch 'x86/asm'")

Cc: <kexec@lists.infradead.org>

Tom Lendacky (2):
  x86/mm, kexec: Fix memory corruption with SME on successive kexecs
  acpi, x86: Remove encryption mask from ACPI page protection type

 arch/x86/include/asm/acpi.h          | 7 ++++++-
 arch/x86/kernel/relocate_kernel_64.S | 7 +++++++
 2 files changed, 13 insertions(+), 1 deletion(-)

-- 
1.9.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v1 1/2] x86/mm, kexec: Fix memory corruption with SME on successive kexecs

[Tom Lendacky]

After issuing successive kexecs it was found that the SHA hash failed
verification when booting the kexec'd kernel.  When SME is enabled, the
change from using pages that were marked encrypted to now being marked as
not encrypted (through new identify mapped page tables) results in memory
corruption if there are any cache entries for the previously encrypted
pages. This is because separate cache entries can exist for the same
physical location but tagged both with and without the encryption bit.

To prevent this, issue a wbinvd before copying the pages from the source
location to the destination location to clear any possible cache entry
conflicts.

Cc: <kexec@lists.infradead.org>
Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
---
 arch/x86/kernel/relocate_kernel_64.S | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S
index 98111b3..c11d8bc 100644
--- a/arch/x86/kernel/relocate_kernel_64.S
+++ b/arch/x86/kernel/relocate_kernel_64.S
@@ -132,6 +132,13 @@ identity_mapped:
 	/* Flush the TLB (needed?) */
 	movq	%r9, %cr3
 
+	/*
+	 * If SME is/was active, there could be old encrypted cache line
+	 * entries that will conflict with the now unencrypted memory
+	 * used by kexec. Flush the caches before copying the kernel.
+	 */
+	wbinvd
+
 	movq	%rcx, %r11
 	call	swap_pages
 
-- 
1.9.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v1 1/2] x86/mm, kexec: Fix memory corruption with SME on successive kexecs

[Ingo Molnar]

* Tom Lendacky <thomas.lendacky@amd.com> wrote:

> After issuing successive kexecs it was found that the SHA hash failed
> verification when booting the kexec'd kernel.  When SME is enabled, the
> change from using pages that were marked encrypted to now being marked as
> not encrypted (through new identify mapped page tables) results in memory
> corruption if there are any cache entries for the previously encrypted
> pages. This is because separate cache entries can exist for the same
> physical location but tagged both with and without the encryption bit.
> 
> To prevent this, issue a wbinvd before copying the pages from the source
> location to the destination location to clear any possible cache entry
> conflicts.
> 
> Cc: <kexec@lists.infradead.org>
> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
> ---
>  arch/x86/kernel/relocate_kernel_64.S | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S
> index 98111b3..c11d8bc 100644
> --- a/arch/x86/kernel/relocate_kernel_64.S
> +++ b/arch/x86/kernel/relocate_kernel_64.S
> @@ -132,6 +132,13 @@ identity_mapped:
>  	/* Flush the TLB (needed?) */
>  	movq	%r9, %cr3
>  
> +	/*
> +	 * If SME is/was active, there could be old encrypted cache line
> +	 * entries that will conflict with the now unencrypted memory
> +	 * used by kexec. Flush the caches before copying the kernel.
> +	 */
> +	wbinvd

WBINVD is very expensive IIRC - several milliseconds.

So if we change the page table from encrypted to unencrypted we need to do a full 
cache flush sounds pretty broken to me - how can then this be done via an API such 
as mmap() without executing WBINVD?

Thanks,

	Ingo

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v1 1/2] x86/mm, kexec: Fix memory corruption with SME on successive kexecs

[Tom Lendacky]

On 7/27/2017 2:17 AM, Ingo Molnar wrote:
> 
> * Tom Lendacky <thomas.lendacky@amd.com> wrote:
> 
>> After issuing successive kexecs it was found that the SHA hash failed
>> verification when booting the kexec'd kernel.  When SME is enabled, the
>> change from using pages that were marked encrypted to now being marked as
>> not encrypted (through new identify mapped page tables) results in memory
>> corruption if there are any cache entries for the previously encrypted
>> pages. This is because separate cache entries can exist for the same
>> physical location but tagged both with and without the encryption bit.
>>
>> To prevent this, issue a wbinvd before copying the pages from the source
>> location to the destination location to clear any possible cache entry
>> conflicts.
>>
>> Cc: <kexec@lists.infradead.org>
>> Signed-off-by: Tom Lendacky <thomas.lendacky@amd.com>
>> ---
>>   arch/x86/kernel/relocate_kernel_64.S | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S
>> index 98111b3..c11d8bc 100644
>> --- a/arch/x86/kernel/relocate_kernel_64.S
>> +++ b/arch/x86/kernel/relocate_kernel_64.S
>> @@ -132,6 +132,13 @@ identity_mapped:
>>   	/* Flush the TLB (needed?) */
>>   	movq	%r9, %cr3
>>   
>> +	/*
>> +	 * If SME is/was active, there could be old encrypted cache line
>> +	 * entries that will conflict with the now unencrypted memory
>> +	 * used by kexec. Flush the caches before copying the kernel.
>> +	 */
>> +	wbinvd
> 
> WBINVD is very expensive IIRC - several milliseconds.
> 
> So if we change the page table from encrypted to unencrypted we need to do a full
> cache flush sounds pretty broken to me - how can then this be done via an API such
> as mmap() without executing WBINVD?

The hardware doesn't enforce coherency between encrypted and unencrypted
mappings of the same physical page[1].  There are APIs that will perform
a targeted cache flush when changing the encryption bit associated with
a page table entry (set_memory_encrypted()/set_memory_decrypted()) and
don't require a full cache flush. But in the case of kexec, there is a
wholesale change of the page tables from what was active to the new
identity mapped tables without any way to know what was previously
mapped and whether it was previously mapped as encrypted or unencrypted.
In this case I don't think an API such as mmap() will help.  For SME, we
will need to be sure the cache is flushed to avoid any coherency issues.

I can #ifdef the wbinvd based on whether AMD_MEM_ENCRYPT is configured
or not so that the wbinvd is avoided if not configured.

Thanks,
Tom

[1] http://support.amd.com/TechDocs/24593.pdf (Section 7.10.6)

> 
> Thanks,
> 
> 	Ingo
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v1 1/2] x86/mm, kexec: Fix memory corruption with SME on successive kexecs

[Linus Torvalds]

On Thu, Jul 27, 2017 at 7:15 AM, Tom Lendacky <thomas.lendacky@amd.com> wrote:
>
> I can #ifdef the wbinvd based on whether AMD_MEM_ENCRYPT is configured
> or not so that the wbinvd is avoided if not configured.

I suspect an ifdef will be useless, since things like distro kernels
tend to enable everything.

So it should probably be disabled dynamically, and only done if the
AMD memory encryption thing has actually been active.

[ There have also been various actual errata with wbinvd, so there
tends to be a non-performance reason to try to avoid it unless
strictly required ]

               Linus

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH v1 1/2] x86/mm, kexec: Fix memory corruption with SME on successive kexecs

[Tom Lendacky]

On 7/27/2017 12:34 PM, Linus Torvalds wrote:
> On Thu, Jul 27, 2017 at 7:15 AM, Tom Lendacky <thomas.lendacky@amd.com> wrote:
>>
>> I can #ifdef the wbinvd based on whether AMD_MEM_ENCRYPT is configured
>> or not so that the wbinvd is avoided if not configured.
> 
> I suspect an ifdef will be useless, since things like distro kernels
> tend to enable everything.
> 
> So it should probably be disabled dynamically, and only done if the
> AMD memory encryption thing has actually been active.
> 
> [ There have also been various actual errata with wbinvd, so there
> tends to be a non-performance reason to try to avoid it unless
> strictly required ]

Ok, I'll make the wbinvd a run time decision based on whether SME is
active at the time.

Thanks,
Tom

> 
>                 Linus
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec