x86: Secure Memory Encryption (SME) fixes 2017-07-26
Subsystems (3)
cover x86: Secure Memory Encryption (SME) fixes 2017-07-26
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'")
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(-)
Signature · 1 line
1.9.1
PATCH 1/2 x86/mm, kexec: Fix memory corruption with SME on successive kexecs
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.
--- arch/x86/kernel/relocate_kernel_64.S | 7 +++++++ 1 file changed, 7 insertions(+)
arch/x86/kernel/relocate_kernel_64.S +7 -0
@@ -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
Signature · 7 lines
1.9.1 _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
Re: [PATCH v1 1/2] x86/mm, kexec: Fix memory corruption with SME on successive kexecs
Quoted 23 lines · 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 <[email protected]>Signed-off-by Tom Lendacky <[email protected]>--- arch/x86/kernel/relocate_kernel_64.S | 7 +++++++ 1 file changed, 7 insertions(+)arch/x86/kernel/relocate_kernel_64.S +0
-0
arch/x86/kernel/relocate_kernel_64.S identity_mapped: · line 140
Quoted 9 lines
arch/x86/kernel/relocate_kernel_64.S +6
-0@@ -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
Re: [PATCH v1 1/2] x86/mm, kexec: Fix memory corruption with SME on successive kexecs
Quoted 38 lines · Ingo Molnar
Quoted 23 lines · 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 <[email protected]>Signed-off-by Tom Lendacky <[email protected]>--- arch/x86/kernel/relocate_kernel_64.S | 7 +++++++ 1 file changed, 7 insertions(+)arch/x86/kernel/relocate_kernel_64.S +0
-0arch/x86/kernel/relocate_kernel_64.S identity_mapped: · line 140
Quoted 9 lines
arch/x86/kernel/relocate_kernel_64.S +6
-0@@ -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. + */ + wbinvdWBINVD is very expensive IIRC - several milliseconds.
Quoted 3 lines
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)
Quoted 5 lines
Thanks,
Ingo
Re: [PATCH v1 1/2] x86/mm, kexec: Fix memory corruption with SME on successive kexecs
Quoted 3 lines · Tom Lendacky
I can #ifdef the wbinvd based on whether
AMD_MEM_ENCRYPTis 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
Re: [PATCH v1 1/2] x86/mm, kexec: Fix memory corruption with SME on successive kexecs
Quoted 8 lines · Linus Torvalds
Quoted 3 lines · Tom Lendacky
I can #ifdef the wbinvd based on whether
AMD_MEM_ENCRYPTis 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.
Quoted 6 lines
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
Quoted 3 lines
Linus
PATCH 2/2 acpi, x86: Remove encryption mask from ACPI page protection type
In reply to Tom LendackyThe function arch_apei_get_mem_attributes() is used to set the page protection type for ACPI physical addresses. When SME is active, the associated protection type needs to not have the encryption mask set since the ACPI tables live in un-encrypted memory. Modify the arch_apei_get_mem_attributes() function to remove the encryption mask when SME is active by returning the PAGE_KERNEL_IO protection type.
--- arch/x86/include/asm/acpi.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
arch/x86/include/asm/acpi.h +6 -1
@@ -34,6 +34,7 @@#ifdef CONFIG_ACPI_APEI # include <asm/pgtable_types.h> +# include <linux/mem_encrypt.h> #endif #ifdef CONFIG_ACPI
@@ -164,8 +165,12 @@ static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)* anything other than PAGE_KERNEL (some arm64 platforms * require the equivalent of PAGE_KERNEL_NOCACHE), return that * until we know differently. +* +* When SME is active, the ACPI information will not reside in +* in memory in an encrypted state so return a protection attribute +* that does not have the encryption bit set. */ - return PAGE_KERNEL; + return sme_active() ? PAGE_KERNEL_IO : PAGE_KERNEL; }#endif
Signature · 1 line
1.9.1
Re: [PATCH v1 2/2] acpi, x86: Remove encryption mask from ACPI page protection type
Quoted 25 lines · Tom Lendacky
The function
arch_apei_get_mem_attributes()is used to set the page protection type for ACPI physical addresses. When SME is active, the associated protection type needs to not have the encryption mask set since the ACPI tables live in un-encrypted memory. Modify thearch_apei_get_mem_attributes()function to remove the encryption mask when SME is active by returning thePAGE_KERNEL_IOprotection type.Signed-off-by Tom Lendacky <[email protected]>--- arch/x86/include/asm/acpi.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)arch/x86/include/asm/acpi.h +1
-0@@ -34,6 +34,7 @@#ifdef CONFIG_ACPI_APEI # include <asm/pgtable_types.h> +# include <linux/mem_encrypt.h> #endif #ifdef CONFIG_ACPI
arch/x86/include/asm/acpi.h static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr) · line 173
Quoted 10 lines
arch/x86/include/asm/acpi.h +5
-1@@ -164,8 +165,12 @@ static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)* anything other than PAGE_KERNEL (some arm64 platforms * require the equivalent of PAGE_KERNEL_NOCACHE), return that * until we know differently. +* +* When SME is active, the ACPI information will not reside in +* in memory in an encrypted state so return a protection attribute +* that does not have the encryption bit set. */ -return PAGE_KERNEL ; +return sme_active () ?PAGE_KERNEL_IO :PAGE_KERNEL ;
'in in memory'?
Also, this seems a bit ad-hoc to me. What are the rules for what is encrypted and what is not?
I presume the main rule is that everything that was written before the kernel activates SME, and which the kernel will read later on, is unencrypted - everything else is encrypted. Is that correct?
How about things like kexec creating a separate mptable for the kexec kernel - would that code have to create unencrypted data? See e820__memblock_alloc_reserved() and related code.
Thanks,
Ingo
Re: [PATCH v1 2/2] acpi, x86: Remove encryption mask from ACPI page protection type
Quoted 26 lines · Ingo Molnar
Quoted 23 lines · Tom Lendacky
The function
arch_apei_get_mem_attributes()is used to set the page protection type for ACPI physical addresses. When SME is active, the associated protection type needs to not have the encryption mask set since the ACPI tables live in un-encrypted memory. Modify thearch_apei_get_mem_attributes()function to remove the encryption mask when SME is active by returning thePAGE_KERNEL_IOprotection type.Signed-off-by Tom Lendacky <[email protected]>--- arch/x86/include/asm/acpi.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)arch/x86/include/asm/acpi.h +1
-0@@ -34,6 +34,7 @@#ifdef CONFIG_ACPI_APEI # include <asm/pgtable_types.h> +# include <linux/mem_encrypt.h> #endif
Quoted 14 lines
arch/x86/include/asm/acpi.h static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr) · line 173
Quoted 12 lines
arch/x86/include/asm/acpi.h +5
-1#ifdef CONFIG_ACPI @@ -164,8 +165,12 @@ static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)* anything other than PAGE_KERNEL (some arm64 platforms * require the equivalent of PAGE_KERNEL_NOCACHE), return that * until we know differently. +* +* When SME is active, the ACPI information will not reside in +* in memory in an encrypted state so return a protection attribute +* that does not have the encryption bit set. */ -return PAGE_KERNEL ; +return sme_active () ?PAGE_KERNEL_IO :PAGE_KERNEL ;'in in memory'?
Should just be one 'in', will fix.
Quoted 7 lines
Also, this seems a bit ad-hoc to me. What are the rules for what is encrypted and what is not?
I presume the main rule is that everything that was written before the kernel activates SME, and which the kernel will read later on, is unencrypted - everything else is encrypted. Is that correct?
Yes, that is the basic rule. Anything from BIOS/UEFI is unencrypted and much of everything else after the kernel encrypts itself will be encrypted. Kernel related hings that aren't be encrypted are, for example, the SWIOTLB bounce buffers, the trampoline area for bringing up APs, etc.
Quoted 4 lines
How about things like kexec creating a separate mptable for the kexec kernel - would that code have to create unencrypted data? See
e820__memblock_alloc_reserved()and related code.
Yes, that would need to be unencrypted since it will be considered to be something supplied by BIOS/UEFI (even though kexec builds it). I'm not familiar with where kexec creates a new mptable. I see the refs to it in e820.c, but I'm missing something. Can you point me to that?
Thanks,
Tom
Quoted 5 lines
Thanks,
Ingo
Borislav Petkov
maintainerRe: [PATCH v1 2/2] acpi, x86: Remove encryption mask from ACPI page protection type
In reply to Tom LendackyQuoted 25 lines · Tom Lendacky
The function
arch_apei_get_mem_attributes()is used to set the page protection type for ACPI physical addresses. When SME is active, the associated protection type needs to not have the encryption mask set since the ACPI tables live in un-encrypted memory. Modify thearch_apei_get_mem_attributes()function to remove the encryption mask when SME is active by returning thePAGE_KERNEL_IOprotection type.Signed-off-by Tom Lendacky <[email protected]>--- arch/x86/include/asm/acpi.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)arch/x86/include/asm/acpi.h +1
-0@@ -34,6 +34,7 @@#ifdef CONFIG_ACPI_APEI # include <asm/pgtable_types.h> +# include <linux/mem_encrypt.h> #endif #ifdef CONFIG_ACPI
arch/x86/include/asm/acpi.h static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr) · line 173
Quoted 10 lines
arch/x86/include/asm/acpi.h +5
-1@@ -164,8 +165,12 @@ static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)* anything other than PAGE_KERNEL (some arm64 platforms * require the equivalent of PAGE_KERNEL_NOCACHE), return that * until we know differently. +* +* When SME is active, the ACPI information will not reside in +* in memory in an encrypted state so return a protection attribute +* that does not have the encryption bit set. */ -return PAGE_KERNEL ; +return sme_active () ?PAGE_KERNEL_IO :PAGE_KERNEL ;
Why isn't there a PAGE_KERNEL_NOENC define which you can simply return instead of testing?
Signature · 5 lines
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--Re: [PATCH v1 2/2] acpi, x86: Remove encryption mask from ACPI page protection type
Quoted 24 lines · Borislav Petkov
Quoted 23 lines · Tom Lendacky
The function
arch_apei_get_mem_attributes()is used to set the page protection type for ACPI physical addresses. When SME is active, the associated protection type needs to not have the encryption mask set since the ACPI tables live in un-encrypted memory. Modify thearch_apei_get_mem_attributes()function to remove the encryption mask when SME is active by returning thePAGE_KERNEL_IOprotection type.Signed-off-by Tom Lendacky <[email protected]>--- arch/x86/include/asm/acpi.h | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)arch/x86/include/asm/acpi.h +1
-0@@ -34,6 +34,7 @@#ifdef CONFIG_ACPI_APEI # include <asm/pgtable_types.h> +# include <linux/mem_encrypt.h> #endif
Quoted 15 lines
arch/x86/include/asm/acpi.h static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr) · line 173
Quoted 12 lines
arch/x86/include/asm/acpi.h +5
-1#ifdef CONFIG_ACPI @@ -164,8 +165,12 @@ static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr)* anything other than PAGE_KERNEL (some arm64 platforms * require the equivalent of PAGE_KERNEL_NOCACHE), return that * until we know differently. +* +* When SME is active, the ACPI information will not reside in +* in memory in an encrypted state so return a protection attribute +* that does not have the encryption bit set. */ -return PAGE_KERNEL ; +return sme_active () ?PAGE_KERNEL_IO :PAGE_KERNEL ;Why isn't there a
PAGE_KERNEL_NOENCdefine which you can simply return instead of testing?
Sounds like something I should add to pgtable_types.h (which has a #define for PAGE_KERNEL_EXEC_NOENC, but not PAGE_KERNEL_NOENC). I'll create that #define.
As for the sme_active() check I was getting ahead of myself since under SEV the encryption mask is needed. I'll change it to just return PAGE_KERNEL_NOENC and then worry about the SEV change in the SEV patches.
Thanks,
Tom
Quoted 1 line
Re: [PATCH v1 2/2] acpi, x86: Remove encryption mask from ACPI page protection type
Quoted 9 lines · Tom Lendacky
Quoted 8 lines
arch/x86/include/asm/acpi.h static inline pgprot_t arch_apei_get_mem_attribute(phys_addr_t addr) · line 173
Quoted 5 lines
arch/x86/include/asm/acpi.h +3
-1+* in memory in an encrypted state so return a protection attribute +* that does not have the encryption bit set. */ -return PAGE_KERNEL ; +return sme_active () ?PAGE_KERNEL_IO :PAGE_KERNEL ;Why isn't there a
PAGE_KERNEL_NOENCdefine which you can simply return instead of testing?
Quoted 8 lines
Sounds like something I should add to
pgtable_types.h(which has a #define forPAGE_KERNEL_EXEC_NOENC, but notPAGE_KERNEL_NOENC). I'll create that #define.As for the
sme_active()check I was getting ahead of myself since under SEV the encryption mask is needed. I'll change it to just returnPAGE_KERNEL_NOENCand then worry about the SEV change in the SEV patches.
Ok, that works for me too, as we at least don't sprinke the code with repeated sme_active() toggles.
Thanks,
Ingo