This patchset fixes dma_mmap_coherent() mapping of unencrypted memory in
otherwise encrypted environments, where it would incorrectly map that memory as
encrypted.
With SEV and sometimes with SME encryption, The dma api coherent memory is
typically unencrypted, meaning the linear kernel map has the encryption
bit cleared. However, default page protection returned from vm_get_page_prot()
has the encryption bit set. So to compute the correct page protection we need
to clear the encryption bit.
Also, in order for the encryption bit setting to survive across do_mmap() and
mprotect_fixup(), We need to make pgprot_modify() aware of it and not touch it.
Therefore make sme_me_mask part of _PAGE_CHG_MASK and make sure
pgprot_modify() preserves also cleared bits that are part of _PAGE_CHG_MASK,
not just set bits. The use of pgprot_modify() is currently quite limited and
easy to audit.
(Note that the encryption status is not logically encoded in the pfn but in
the page protection even if an address line in the physical address is used).
The patchset has seen some sanity testing by exporting dma_pgprot() and
using it in the vmwgfx mmap handler with SEV enabled.
As far as I can tell there are no current users of dma_mmap_coherent() with
SEV or SME encryption which means that there is no need to CC stable.
Changes since:
RFC:
- Make sme_me_mask port of _PAGE_CHG_MASK rather than using it by its own in
pgprot_modify().
v2:
- Clarify which use-cases this patchset actually fixes.
Cc: Dave Hansen <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Christian König <[email protected]>
Cc: Marek Szyprowski <[email protected]>
Cc: Tom Lendacky <[email protected]>
From: Thomas Hellstrom <[email protected]>
When dma_mmap_coherent() sets up a mapping to unencrypted coherent memory
under SEV encryption and sometimes under SME encryption, it will actually
set up an encrypted mapping rather than an unencrypted, causing devices
that DMAs from that memory to read encrypted contents. Fix this.
When force_dma_unencrypted() returns true, the linear kernel map of the
coherent pages have had the encryption bit explicitly cleared and the
page content is unencrypted. Make sure that any additional PTEs we set
up to these pages also have the encryption bit cleared by having
dma_pgprot() return a protection with the encryption bit cleared in this
case.
Cc: Dave Hansen <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Christian König <[email protected]>
Cc: Marek Szyprowski <[email protected]>
Cc: Tom Lendacky <[email protected]>
Signed-off-by: Thomas Hellstrom <[email protected]>
---
kernel/dma/mapping.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/kernel/dma/mapping.c b/kernel/dma/mapping.c
index b0038ca3aa92..2b499dcae74f 100644
--- a/kernel/dma/mapping.c
+++ b/kernel/dma/mapping.c
@@ -157,6 +157,8 @@ EXPORT_SYMBOL(dma_get_sgtable_attrs);
*/
pgprot_t dma_pgprot(struct device *dev, pgprot_t prot, unsigned long attrs)
{
+ if (force_dma_unencrypted(dev))
+ prot = pgprot_decrypted(prot);
if (dev_is_dma_coherent(dev) ||
(IS_ENABLED(CONFIG_DMA_NONCOHERENT_CACHE_SYNC) &&
(attrs & DMA_ATTR_NON_CONSISTENT)))
--
2.20.1
From: Thomas Hellstrom <[email protected]>
When SEV or SME is enabled and active, vm_get_page_prot() typically
returns with the encryption bit set. This means that users of
pgprot_modify(, vm_get_page_prot()) (mprotect_fixup, do_mmap) end up with
a value of vma->vm_pg_prot that is not consistent with the intended
protection of the PTEs. This is also important for fault handlers that
rely on the VMA vm_page_prot to set the page protection. Fix this by
not allowing pgprot_modify() to change the encryption bit, similar to
how it's done for PAT bits.
Cc: Dave Hansen <[email protected]>
Cc: Andy Lutomirski <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Thomas Gleixner <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Borislav Petkov <[email protected]>
Cc: "H. Peter Anvin" <[email protected]>
Cc: Christoph Hellwig <[email protected]>
Cc: Christian König <[email protected]>
Cc: Marek Szyprowski <[email protected]>
Cc: Tom Lendacky <[email protected]>
Signed-off-by: Thomas Hellstrom <[email protected]>
---
arch/x86/include/asm/pgtable.h | 7 +++++--
arch/x86/include/asm/pgtable_types.h | 2 +-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
index 0bc530c4eb13..1e6bb4c25334 100644
--- a/arch/x86/include/asm/pgtable.h
+++ b/arch/x86/include/asm/pgtable.h
@@ -624,12 +624,15 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
return __pmd(val);
}
-/* mprotect needs to preserve PAT bits when updating vm_page_prot */
+/*
+ * mprotect needs to preserve PAT and encryption bits when updating
+ * vm_page_prot
+ */
#define pgprot_modify pgprot_modify
static inline pgprot_t pgprot_modify(pgprot_t oldprot, pgprot_t newprot)
{
pgprotval_t preservebits = pgprot_val(oldprot) & _PAGE_CHG_MASK;
- pgprotval_t addbits = pgprot_val(newprot);
+ pgprotval_t addbits = pgprot_val(newprot) & ~_PAGE_CHG_MASK;
return __pgprot(preservebits | addbits);
}
diff --git a/arch/x86/include/asm/pgtable_types.h b/arch/x86/include/asm/pgtable_types.h
index b5e49e6bac63..e13084b3d6cb 100644
--- a/arch/x86/include/asm/pgtable_types.h
+++ b/arch/x86/include/asm/pgtable_types.h
@@ -123,7 +123,7 @@
*/
#define _PAGE_CHG_MASK (PTE_PFN_MASK | _PAGE_PCD | _PAGE_PWT | \
_PAGE_SPECIAL | _PAGE_ACCESSED | _PAGE_DIRTY | \
- _PAGE_SOFT_DIRTY | _PAGE_DEVMAP)
+ _PAGE_SOFT_DIRTY | _PAGE_DEVMAP | sme_me_mask)
#define _HPAGE_CHG_MASK (_PAGE_CHG_MASK | _PAGE_PSE)
/*
--
2.20.1
On 9/11/19 7:40 AM, Thomas Hellström (VMware) wrote:
> From: Thomas Hellstrom <[email protected]>
>
> When SEV or SME is enabled and active, vm_get_page_prot() typically
> returns with the encryption bit set. This means that users of
> pgprot_modify(, vm_get_page_prot()) (mprotect_fixup, do_mmap) end up with
> a value of vma->vm_pg_prot that is not consistent with the intended
> protection of the PTEs. This is also important for fault handlers that
> rely on the VMA vm_page_prot to set the page protection. Fix this by
> not allowing pgprot_modify() to change the encryption bit, similar to
> how it's done for PAT bits.
>
> Cc: Dave Hansen <[email protected]>
> Cc: Andy Lutomirski <[email protected]>
> Cc: Peter Zijlstra <[email protected]>
> Cc: Thomas Gleixner <[email protected]>
> Cc: Ingo Molnar <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> Cc: "H. Peter Anvin" <[email protected]>
> Cc: Christoph Hellwig <[email protected]>
> Cc: Christian König <[email protected]>
> Cc: Marek Szyprowski <[email protected]>
> Cc: Tom Lendacky <[email protected]>
> Signed-off-by: Thomas Hellstrom <[email protected]>
> ---
> arch/x86/include/asm/pgtable.h | 7 +++++--
> arch/x86/include/asm/pgtable_types.h | 2 +-
> 2 files changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
> index 0bc530c4eb13..1e6bb4c25334 100644
> --- a/arch/x86/include/asm/pgtable.h
> +++ b/arch/x86/include/asm/pgtable.h
> @@ -624,12 +624,15 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
> return __pmd(val);
> }
>
> -/* mprotect needs to preserve PAT bits when updating vm_page_prot */
> +/*
> + * mprotect needs to preserve PAT and encryption bits when updating
> + * vm_page_prot
> + */
> #define pgprot_modify pgprot_modify
> static inline pgprot_t pgprot_modify(pgprot_t oldprot, pgprot_t newprot)
> {
> pgprotval_t preservebits = pgprot_val(oldprot) & _PAGE_CHG_MASK;
> - pgprotval_t addbits = pgprot_val(newprot);
> + pgprotval_t addbits = pgprot_val(newprot) & ~_PAGE_CHG_MASK;
> return __pgprot(preservebits | addbits);
> }
>
> diff --git a/arch/x86/include/asm/pgtable_types.h b/arch/x86/include/asm/pgtable_types.h
> index b5e49e6bac63..e13084b3d6cb 100644
> --- a/arch/x86/include/asm/pgtable_types.h
> +++ b/arch/x86/include/asm/pgtable_types.h
> @@ -123,7 +123,7 @@
> */
> #define _PAGE_CHG_MASK (PTE_PFN_MASK | _PAGE_PCD | _PAGE_PWT | \
> _PAGE_SPECIAL | _PAGE_ACCESSED | _PAGE_DIRTY | \
> - _PAGE_SOFT_DIRTY | _PAGE_DEVMAP)
> + _PAGE_SOFT_DIRTY | _PAGE_DEVMAP | sme_me_mask)
There is a _PAGE_ENC definition that you could use to make this more
consistent with the current definition.
Thanks,
Tom
> #define _HPAGE_CHG_MASK (_PAGE_CHG_MASK | _PAGE_PSE)
>
> /*
>
On 9/11/19 3:27 PM, Lendacky, Thomas wrote:
> On 9/11/19 7:40 AM, Thomas Hellström (VMware) wrote:
>> From: Thomas Hellstrom <[email protected]>
>>
>> When SEV or SME is enabled and active, vm_get_page_prot() typically
>> returns with the encryption bit set. This means that users of
>> pgprot_modify(, vm_get_page_prot()) (mprotect_fixup, do_mmap) end up with
>> a value of vma->vm_pg_prot that is not consistent with the intended
>> protection of the PTEs. This is also important for fault handlers that
>> rely on the VMA vm_page_prot to set the page protection. Fix this by
>> not allowing pgprot_modify() to change the encryption bit, similar to
>> how it's done for PAT bits.
>>
>> Cc: Dave Hansen <[email protected]>
>> Cc: Andy Lutomirski <[email protected]>
>> Cc: Peter Zijlstra <[email protected]>
>> Cc: Thomas Gleixner <[email protected]>
>> Cc: Ingo Molnar <[email protected]>
>> Cc: Borislav Petkov <[email protected]>
>> Cc: "H. Peter Anvin" <[email protected]>
>> Cc: Christoph Hellwig <[email protected]>
>> Cc: Christian König <[email protected]>
>> Cc: Marek Szyprowski <[email protected]>
>> Cc: Tom Lendacky <[email protected]>
>> Signed-off-by: Thomas Hellstrom <[email protected]>
>> ---
>> arch/x86/include/asm/pgtable.h | 7 +++++--
>> arch/x86/include/asm/pgtable_types.h | 2 +-
>> 2 files changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/x86/include/asm/pgtable.h b/arch/x86/include/asm/pgtable.h
>> index 0bc530c4eb13..1e6bb4c25334 100644
>> --- a/arch/x86/include/asm/pgtable.h
>> +++ b/arch/x86/include/asm/pgtable.h
>> @@ -624,12 +624,15 @@ static inline pmd_t pmd_modify(pmd_t pmd, pgprot_t newprot)
>> return __pmd(val);
>> }
>>
>> -/* mprotect needs to preserve PAT bits when updating vm_page_prot */
>> +/*
>> + * mprotect needs to preserve PAT and encryption bits when updating
>> + * vm_page_prot
>> + */
>> #define pgprot_modify pgprot_modify
>> static inline pgprot_t pgprot_modify(pgprot_t oldprot, pgprot_t newprot)
>> {
>> pgprotval_t preservebits = pgprot_val(oldprot) & _PAGE_CHG_MASK;
>> - pgprotval_t addbits = pgprot_val(newprot);
>> + pgprotval_t addbits = pgprot_val(newprot) & ~_PAGE_CHG_MASK;
>> return __pgprot(preservebits | addbits);
>> }
>>
>> diff --git a/arch/x86/include/asm/pgtable_types.h b/arch/x86/include/asm/pgtable_types.h
>> index b5e49e6bac63..e13084b3d6cb 100644
>> --- a/arch/x86/include/asm/pgtable_types.h
>> +++ b/arch/x86/include/asm/pgtable_types.h
>> @@ -123,7 +123,7 @@
>> */
>> #define _PAGE_CHG_MASK (PTE_PFN_MASK | _PAGE_PCD | _PAGE_PWT | \
>> _PAGE_SPECIAL | _PAGE_ACCESSED | _PAGE_DIRTY | \
>> - _PAGE_SOFT_DIRTY | _PAGE_DEVMAP)
>> + _PAGE_SOFT_DIRTY | _PAGE_DEVMAP | sme_me_mask)
> There is a _PAGE_ENC definition that you could use to make this more
> consistent with the current definition.
Ah yes.
I'll wait a bit to see if there are more concerns and include this in
the next version.
Thanks,
Thomas
>
> Thanks,
> Tom
>
>> #define _HPAGE_CHG_MASK (_PAGE_CHG_MASK | _PAGE_PSE)
>>
>> /*
>>