linux-kernel - Re: [PATCH v2 2/3] x86/mm: add .data..decrypted section to hold shared variables

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]

Message-ID: <0de361d4-1d1e-0584-f9d4-41bf5936e2be@amd.com>
Date:   Wed, 29 Aug 2018 10:33:24 -0500
From:   Brijesh Singh <brijesh.singh@....com>
To:     Sean Christopherson <sean.j.christopherson@...el.com>
Cc:     brijesh.singh@....com, x86@...nel.org,
        linux-kernel@...r.kernel.org, kvm@...r.kernel.org,
        stable@...r.kernel.org, Tom Lendacky <thomas.lendacky@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Borislav Petkov <bp@...e.de>, "H. Peter Anvin" <hpa@...or.com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Radim Krčmář <rkrcmar@...hat.com>
Subject: Re: [PATCH v2 2/3] x86/mm: add .data..decrypted section to hold
 shared variables



On 08/29/2018 10:03 AM, Sean Christopherson wrote:
> On Tue, Aug 28, 2018 at 05:12:56PM -0500, Brijesh Singh wrote:
>> kvmclock defines few static variables which are shared with hypervisor
>> during the kvmclock initialization.
>>
>> When SEV is active, memory is encrypted with a guest-specific key, and
>> if guest OS wants to share the memory region with hypervisor then it must
>> clear the C-bit before sharing it. Currently, we use
>> kernel_physical_mapping_init() to split large pages before clearing the
>> C-bit on shared pages. But the kernel_physical_mapping_init fails when
>> called from the kvmclock initialization (mainly because memblock allocator
>> was not ready).
>>
>> The '__decrypted' can be used to define a shared variable; the variables
>> will be put in the .data.decryption section. This section is mapped with
>> C=0 early in the boot, we also ensure that the initialized values are
>> updated to match with C=0 (i.e perform an in-place decryption). The
>> .data..decrypted section is PMD aligned and sized so that we avoid the
>> need to split the large pages when mapping this section.
> 
> What about naming the attribute (and section) '__unencrypted' instead
> of '__decrypted'?  The attribute should be a property describing how
> the data must be accessed, it shouldn't imply anything regarding the
> history of the data.  Decrypted implies that data was once encrypted,
> whereas unencrypted simply states that the data is stored in plain
> text.  All data that has been decrypted is also unencrypted, but the
> reverse does not hold true.
> 


During the initial SEV/SME patch review cycle we had some discussion 
about using decrypted vs unencrypted. At that time the consensus was
that a memory range mapped with C=0 should be referred as 'decrypted'.
Having said so, I do see your point and I am not oppose to calling it
'unencrypted' if others agrees to it.

Tom and Boris, thoughts ?