linux-kernel - Re: [PATCH] x86/efi: Access EFI MMIO data as unencrypted when SEV is active

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

Date:   Tue, 3 Jul 2018 17:37:18 +0200
From:   Ard Biesheuvel <ard.biesheuvel@...aro.org>
To:     Brijesh Singh <brijesh.singh@....com>
Cc:     "the arch/x86 maintainers" <x86@...nel.org>,
        linux-efi <linux-efi@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Tom Lendacky <thomas.lendacky@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...e.de>,
        KVM devel mailing list <kvm@...r.kernel.org>,
        Matt Fleming <matt@...eblueprint.co.uk>,
        Andy Lutomirski <luto@...nel.org>,
        "# 4 . 15 . x" <stable@...r.kernel.org>
Subject: Re: [PATCH] x86/efi: Access EFI MMIO data as unencrypted when SEV is active

On 3 July 2018 at 15:32, Brijesh Singh <brijesh.singh@....com> wrote:
> SEV guest fails to update the UEFI runtime variables stored in the
> flash. commit 1379edd59673 ("x86/efi: Access EFI data as encrypted
> when SEV is active") unconditionally maps all the UEFI runtime data
> as 'encrypted' (C=1). When SEV is active the UEFI runtime data marked
> as EFI_MEMORY_MAPPED_IO should be mapped as 'unencrypted' so that both
> guest and hypervisor can access the data.
>

I'm uncomfortable having to carry these heuristics in the kernel. The
UEFI memory map should be the definitive source of information
regarding how the OS should map the regions it describes, and if we
need to guess the encryption status, we are likely to get it wrong at
least some of the times.

Is any work underway to get the UEFI spec extended to take encrypted
memory into account? I am aware that we cannot disclose specifics, but
you should be able to disclose whether it is under discussion or not.

In the mean time, we will have to do something, I know that, but I
would like to discuss the proper solution before proceeding with the
stop gap one.

-- 
Ard.


> Fixes: 1379edd59673 (x86/efi: Access EFI data as encrypted ...)
> Cc: Tom Lendacky <thomas.lendacky@....com>
> Cc: Thomas Gleixner <tglx@...utronix.de>
> Cc: Borislav Petkov <bp@...e.de>
> Cc: linux-efi@...r.kernel.org
> Cc: kvm@...r.kernel.org
> Cc: Ard Biesheuvel <ard.biesheuvel@...aro.org>
> Cc: Matt Fleming <matt@...eblueprint.co.uk>
> Cc: Andy Lutomirski <luto@...nel.org>
> Cc: <stable@...r.kernel.org> # 4.15.x
> Signed-off-by: Brijesh Singh <brijesh.singh@....com>
> ---
>  arch/x86/platform/efi/efi_64.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c
> index 77873ce..5f2eb32 100644
> --- a/arch/x86/platform/efi/efi_64.c
> +++ b/arch/x86/platform/efi/efi_64.c
> @@ -417,7 +417,7 @@ static void __init __map_region(efi_memory_desc_t *md, u64 va)
>         if (!(md->attribute & EFI_MEMORY_WB))
>                 flags |= _PAGE_PCD;
>
> -       if (sev_active())
> +       if (sev_active() && md->type != EFI_MEMORY_MAPPED_IO)
>                 flags |= _PAGE_ENC;
>
>         pfn = md->phys_addr >> PAGE_SHIFT;
> --
> 2.7.4
>