linux-kernel - Re: [PATCH v6 19/42] x86/mm: Add support to validate memory when changing C-bit

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:   Thu, 11 Nov 2021 08:49:49 -0600
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Borislav Petkov <bp@...en8.de>,
        Brijesh Singh <brijesh.singh@....com>
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>,
        "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>,
        Dov Murik <dovmurik@...ux.ibm.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>,
        tony.luck@...el.com, marcorr@...gle.com,
        sathyanarayanan.kuppuswamy@...ux.intel.com
Subject: Re: [PATCH v6 19/42] x86/mm: Add support to validate memory when
 changing C-bit

On 11/10/21 12:43 PM, Borislav Petkov wrote:
> On Wed, Nov 10, 2021 at 08:21:21AM -0600, Brijesh Singh wrote:
>> I am assuming you mean add some compile time check to ensure that desc will
>> fit in the shared buffer ?
> 
> No:
> 
> struct ghcb {
> 
> 	...
> 
>          u8 shared_buffer[2032];
> 
> so that memcpy needs to do:
> 
> 	memcpy(ghcb->shared_buffer, desc, min_t(int, 2032, sizeof(*desc)));
> 
> with that 2032 behind a proper define, ofc.

2032 => sizeof(ghcb->shared_buffer) ?

The idea is that a full snp_psc_desc structure is meant to fit completely 
in the shared_buffer area. So if there are no compile time checks, then 
the code on the HV side will need to ensure that the input doesn't cause 
the HV to access the structure outside of the shared_buffer area - which, 
IIRC, it does (think protect against a malicious guest), so the min_t() on 
the memcpy should be safe on the guest side.

But given the snp_psc_desc is sized/meant to fit completely in the 
shared_buffer, a compile time check would be a good idea, too, right?

Thanks,
Tom

> 
>> I can drop the overlap comment to avoid the confusion, as you pointed it
>> more of the future thing. Basically overlap is the below condition
>>
>> set_memory_private(gfn=0, page_size=2m)
>> set_memory_private(gfn=10, page_size=4k)
>>
>> The RMPUPDATE instruction will detect overlap on the second call and return
>> an error to the guest. After we add the support to track the page validation
>> state (either in bitmap or page flag), the second call will not be issued
>> and thus avoid an overlap errors. For now, we use the page_size=4k for all
>> the page state changes from the kernel.
> 
> Yah, sounds like the comment is not needed now. You could put this in
> the commit message, though.
> 
> Thx.
>