Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Thu, 11 Aug 2022 13:26:40 +0200
From:   Borislav Petkov <bp@alien8.de>
To:     Dave Hansen <dave.hansen@intel.com>
Cc:     "Kirill A. Shutemov" <kirill.shutemov@linux.intel.com>,
        Andy Lutomirski <luto@kernel.org>,
        Sean Christopherson <seanjc@google.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Joerg Roedel <jroedel@suse.de>,
        Ard Biesheuvel <ardb@kernel.org>,
        Andi Kleen <ak@linux.intel.com>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@linux.intel.com>,
        David Rientjes <rientjes@google.com>,
        Vlastimil Babka <vbabka@suse.cz>,
        Tom Lendacky <thomas.lendacky@amd.com>,
        Thomas Gleixner <tglx@linutronix.de>,
        Peter Zijlstra <peterz@infradead.org>,
        Paolo Bonzini <pbonzini@redhat.com>,
        Ingo Molnar <mingo@redhat.com>,
        Varad Gautam <varad.gautam@suse.com>,
        Dario Faggioli <dfaggioli@suse.com>,
        Mike Rapoport <rppt@kernel.org>,
        David Hildenbrand <david@redhat.com>,
        marcelo.cerri@canonical.com, tim.gardner@canonical.com,
        khalid.elmously@canonical.com, philip.cox@canonical.com,
        x86@kernel.org, linux-mm@kvack.org, linux-coco@lists.linux.dev,
        linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCHv7 10/14] x86/mm: Avoid load_unaligned_zeropad() stepping
 into unaccepted memory
Message-ID: <YvTncOa6KSr8EIuE@zn.tnic>
References: <20220614120231.48165-1-kirill.shutemov@linux.intel.com>
 <20220614120231.48165-11-kirill.shutemov@linux.intel.com>
 <Yt/ANO5usdV+JSSW@zn.tnic>
 <80cc204b-a24f-684f-ec66-1361b69cae39@intel.com>
 <073c5a97-272c-c5a0-19f2-c3f14f916c72@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <073c5a97-272c-c5a0-19f2-c3f14f916c72@intel.com>
Precedence: bulk

On Wed, Aug 03, 2022 at 07:02:31AM -0700, Dave Hansen wrote:
> One other thing I remembered as I re-read my write up on this.
> 
> In the "new" mode, guests never get #VE's for unaccepted memory.  They
> just exit to the host and can never be reentered.  They must be killed.

Yeah, this is the part which I think is really silly.

OSes, in their execution lifetime, can - erroneously or not - but it
happens pretty often in real life, touch some unrelated memory. And this
has never been a big deal - #PF, that's it.

But now they don't even get a chance to correct their mistake - VMEXIT,
die.

load_unaligned_zeropad() is just one case.

Imagine the user loads some buggy driver in the guest and that driver
starts doing stray memory accesses through a wild pointer into the
fields. Guest dies immediately.

Dunno bit it all feels a bit too harsh and unfriendly to me.

Sure, if that user is really unlucky, those stray accesses can kill
his OS on baremetal too. So maybe you could argue here that such stray
accesses are actually a good thing. :)

All I know is, there should be a more resilient way to handle those.

> In the "old" mode, I _believe_ that the guest always gets a #VE for
> non-EPT-present memory.  The #VE is basically the same no matter if the
> page is unaccepted or if the host goes out and makes a
> previously-accepted page non-present.
> 
> One really nasty implication of this "old" mode is that the host can
> remove *accepted* pages that are used in the syscall gap.  That means
> that the #VE handler would need to be of the paranoid variety which
> opens up all kinds of other fun.

Yeah, I believe this needs to be dealt with anyway, for SNP at least.
But on AMD it would simply cause an exception and it'll be handled in
the #VC thing. And there's some ugly code to deal with the gap too.

>  * "Old" - #VE's can happen in the syscall gap
>  * "New" - #VE's happen at better-defined times.  Unexpected ones are
>    fatal.
> 
> There's a third option which I proposed but doesn't yet exist.  The TDX
> module _could_ separate the behavior of unaccepted memory #VE's and
> host-induced #VEs.  This way, we could use load_unaligned_zeropad() with
> impunity and handle it in the #VE handler.  At the same time, the host
> would not be allowed to remove accepted memory and cause problems in the
> syscall gap.  Kinda the best of both worlds.

I like that. This should've been the default from the get-go. Oh well,
what's it called in English, hindsight is 20 20...?

> But, I'm not sure how valuable that would be now that we have the
> (admittedly squirrelly) code to avoid load_unaligned_zeropad() #VE's.

I think you should push for the bestest solution and one day we can kill
those ugly workarounds.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette