linux-kernel - Re: [PATCH v9 42/43] virt: sevguest: Add support to derive key

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, 8 Feb 2022 09:56:52 +0200
From:   Dov Murik <dovmurik@...ux.ibm.com>
To:     Brijesh Singh <brijesh.singh@....com>,
        Borislav Petkov <bp@...en8.de>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        linux-efi@...r.kernel.org, platform-driver-x86@...r.kernel.org,
        linux-coco@...ts.linux.dev, linux-mm@...ck.org,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Joerg Roedel <jroedel@...e.de>,
        Tom Lendacky <thomas.lendacky@....com>,
        "H. Peter Anvin" <hpa@...or.com>, Ard Biesheuvel <ardb@...nel.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Sean Christopherson <seanjc@...gle.com>,
        Vitaly Kuznetsov <vkuznets@...hat.com>,
        Jim Mattson <jmattson@...gle.com>,
        Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Sergio Lopez <slp@...hat.com>, Peter Gonda <pgonda@...gle.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        David Rientjes <rientjes@...gle.com>,
        Tobin Feldman-Fitzthum <tobin@....com>,
        Michael Roth <michael.roth@....com>,
        Vlastimil Babka <vbabka@...e.cz>,
        "Kirill A . Shutemov" <kirill@...temov.name>,
        Andi Kleen <ak@...ux.intel.com>,
        "Dr . David Alan Gilbert" <dgilbert@...hat.com>,
        brijesh.ksingh@...il.com, tony.luck@...el.com, marcorr@...gle.com,
        sathyanarayanan.kuppuswamy@...ux.intel.com,
        Liam Merwick <liam.merwick@...cle.com>,
        Dov Murik <dovmurik@...ux.ibm.com>
Subject: Re: [PATCH v9 42/43] virt: sevguest: Add support to derive key



On 07/02/2022 22:08, Brijesh Singh wrote:
> 
> 
> On 2/7/22 1:09 PM, Dov Murik wrote:
>>
>>
>> On 07/02/2022 18:23, Brijesh Singh wrote:
>>>
>>>
>>> On 2/7/22 2:52 AM, Borislav Petkov wrote:
>>>> Those are allocated on stack, why are you clearing them?
>>>
>>> Yep, no need to explicitly clear it. I'll take it out in next rev.
>>>
>>
>> Wait, this is key material generated by PSP and passed to userspace.
>> Why leave copies of it floating around kernel memory?  I thought that's
>> the whole reason for these memzero_explicit() calls (maybe add a
>> comment?).
>>
> 
> 
> Ah, now I remember I added the memzero_explicit() to address your review
> feedback :) In that patch version, we were using the kmalloc() to store
> the response data; since then, we switched to stack. We will leak the
> key outside when the stack is converted private-> shared; I don't know
> if any of these are going to happen. I can add a comment and keep the
> memzero_explicit() call.
> 

Just to be clear, I didn't mean necessarily "leak the key to the
untrusted host" (even if a page is converted back from private to
shared, it is encrypted, so host can't read its contents).  But even
*inside* the guest, when dealing with sensitive data like keys, we
should minimize the amount of copies that float around (I assume this is
the reason for most of the uses of memzero_explicit() in the kernel).

-Dov



> Boris, let me know if you are okay with it?
> 
> 
>> As an example, in arch/x86/crypto/aesni-intel_glue.c there are two calls
>> to memzero_explicit(), both on stack variables; the only reason for
>> these calls (as I understand it) is to avoid some future possible leak
>> of this sensitive data (keys, cipher context, etc.).  I'm sure there are
>> other examples in the kernel code.
>>