linux-kernel - Re: [PATCHv3 2/2] x86/mm: Make memremap(MEMREMAP

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] [day] [month] [year] [list]

Message-ID: <557ecd25-039d-777d-c4ee-f2e83ed8d2dc@amd.com>
Date: Tue, 14 Jan 2025 09:06:35 -0600
From: Tom Lendacky <thomas.lendacky@....com>
To: "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>
Cc: Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
 Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
 x86@...nel.org, "H. Peter Anvin" <hpa@...or.com>,
 Andy Lutomirski <luto@...nel.org>, Albert Ou <aou@...s.berkeley.edu>,
 Alexei Starovoitov <ast@...nel.org>, Andrea Parri <parri.andrea@...il.com>,
 Arnd Bergmann <arnd@...db.de>, Daniel Borkmann <daniel@...earbox.net>,
 Eric Chan <ericchancf@...gle.com>, Jason Gunthorpe <jgg@...pe.ca>,
 Kai Huang <kai.huang@...el.com>, Kefeng Wang <wangkefeng.wang@...wei.com>,
 Kent Overstreet <kent.overstreet@...ux.dev>,
 Palmer Dabbelt <palmer@...osinc.com>,
 Paul Walmsley <paul.walmsley@...ive.com>,
 Russell King <linux@...linux.org.uk>,
 Samuel Holland <samuel.holland@...ive.com>,
 Suren Baghdasaryan <surenb@...gle.com>, Yuntao Wang <ytcoode@...il.com>,
 linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
 linux-riscv@...ts.infradead.org, stable@...r.kernel.org,
 Ashish Kalra <ashish.kalra@....com>, "Maciej W. Rozycki" <macro@...am.me.uk>
Subject: Re: [PATCHv3 2/2] x86/mm: Make memremap(MEMREMAP_WB) map memory as
 encrypted by default

On 1/14/25 08:44, Kirill A. Shutemov wrote:
> On Tue, Jan 14, 2025 at 08:33:39AM -0600, Tom Lendacky wrote:
>> On 1/14/25 01:27, Kirill A. Shutemov wrote:
>>> On Mon, Jan 13, 2025 at 02:47:56PM -0600, Tom Lendacky wrote:
>>>> On 1/13/25 07:14, Kirill A. Shutemov wrote:
>>>>> Currently memremap(MEMREMAP_WB) can produce decrypted/shared mapping:
>>>>>
>>>>> memremap(MEMREMAP_WB)
>>>>>   arch_memremap_wb()
>>>>>     ioremap_cache()
>>>>>       __ioremap_caller(.encrytped = false)
>>>>>
>>>>> In such cases, the IORES_MAP_ENCRYPTED flag on the memory will determine
>>>>> if the resulting mapping is encrypted or decrypted.
>>>>>
>>>>> Creating a decrypted mapping without explicit request from the caller is
>>>>> risky:
>>>>>
>>>>>   - It can inadvertently expose the guest's data and compromise the
>>>>>     guest.
>>>>>
>>>>>   - Accessing private memory via shared/decrypted mapping on TDX will
>>>>>     either trigger implicit conversion to shared or #VE (depending on
>>>>>     VMM implementation).
>>>>>
>>>>>     Implicit conversion is destructive: subsequent access to the same
>>>>>     memory via private mapping will trigger a hard-to-debug #VE crash.
>>>>>
>>>>> The kernel already provides a way to request decrypted mapping
>>>>> explicitly via the MEMREMAP_DEC flag.
>>>>>
>>>>> Modify memremap(MEMREMAP_WB) to produce encrypted/private mapping by
>>>>> default unless MEMREMAP_DEC is specified.
>>>>>
>>>>> Fix the crash due to #VE on kexec in TDX guests if CONFIG_EISA is enabled.
>>>>
>>>> This patch causes my bare-metal system to crash during boot when using
>>>> mem_encrypt=on:
>>>>
>>>> [    2.392934] efi: memattr: Entry type should be RuntimeServiceCode/Data
>>>> [    2.393731] efi: memattr: ! 0x214c42f01f1162a-0xee70ac7bd1a9c629 [type=2028324321|attr=0x6590648fa4209879]
>>>
>>> Could you try if this helps?
>>>
>>> diff --git a/drivers/firmware/efi/memattr.c b/drivers/firmware/efi/memattr.c
>>> index c38b1a335590..b5051dcb7c1d 100644
>>> --- a/drivers/firmware/efi/memattr.c
>>> +++ b/drivers/firmware/efi/memattr.c
>>> @@ -160,7 +160,7 @@ int __init efi_memattr_apply_permissions(struct mm_struct *mm,
>>>  	if (WARN_ON(!efi_enabled(EFI_MEMMAP)))
>>>  		return 0;
>>>  
>>> -	tbl = memremap(efi_mem_attr_table, tbl_size, MEMREMAP_WB);
>>> +	tbl = memremap(efi_mem_attr_table, tbl_size, MEMREMAP_WB | MEMREMAP_DEC);
>>
>> Well that would work for SME where EFI tables/data are not encrypted,
>> but will break for SEV where EFI tables/data are encrypted.
> 
> Hm. Why would it break for SEV? It brings the situation back to what it
> was before the patch.

Ah, true. I can try it and see how much further SME gets. Hopefully it
doesn't turn into a whack-a-mole thing.

Thanks,
Tom

> 
> Note that that __ioremap_caller() would still check io_desc.flags before
> mapping it as decrypted.
>