linux-kernel - Re: [PATCHv4 3/8] efi/x86: Implement support for unaccepted memory

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:   Mon, 18 Apr 2022 18:38:52 +0200
From:   Borislav Petkov <bp@...en8.de>
To:     "Kirill A. Shutemov" <kirill@...temov.name>
Cc:     "Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
        Andy Lutomirski <luto@...nel.org>,
        Sean Christopherson <seanjc@...gle.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Joerg Roedel <jroedel@...e.de>,
        Ard Biesheuvel <ardb@...nel.org>,
        Andi Kleen <ak@...ux.intel.com>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>,
        David Rientjes <rientjes@...gle.com>,
        Vlastimil Babka <vbabka@...e.cz>,
        Tom Lendacky <thomas.lendacky@....com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Peter Zijlstra <peterz@...radead.org>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Ingo Molnar <mingo@...hat.com>,
        Varad Gautam <varad.gautam@...e.com>,
        Dario Faggioli <dfaggioli@...e.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Brijesh Singh <brijesh.singh@....com>,
        Mike Rapoport <rppt@...nel.org>,
        David Hildenbrand <david@...hat.com>, x86@...nel.org,
        linux-mm@...ck.org, linux-coco@...ts.linux.dev,
        linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCHv4 3/8] efi/x86: Implement support for unaccepted memory

On Mon, Apr 18, 2022 at 06:55:45PM +0300, Kirill A. Shutemov wrote:
> I'm confused here. What is wrong with linux/ include namespace?

The problem is that you need all kinds of workarounds so that the
decompressor builds. Just look at the beginning of

arch/x86/boot/compressed/misc.h

Even you had to do them:

/* cpu_feature_enabled() cannot be used this early */
#define USE_EARLY_PGTABLE_L5

That thing sprinkled everywhere is not a clean solution.

> Yes, we had story with <asm/io.h> that actually caused issue in
> decompression code, but linux/ has a lot of perfectly portable
> library-like stuff.

Yes, those are fine except that not everything that leaks into the
decompressor code through includes is perfectly portable.

> Could you explain what rules are?

Library-like stuff like types.h, linkage.h, etc we could include for now
but including linux/kernel.h which pulls in everything but the kitchen
sink is bad.

So I'd like for the decompressor to be completely separate from kernel
proper because it is a whole different thing and I want for us to be
able to include headers in it without ugly workarounds just so that
kernel proper include changes do not influence the decompressor.

> Hm. accept_or_mark_unaccepted()?

What's wrong with early_accept_memory()?

> > Immediately? As opposed to delayed?
> 
> Yes. Otherwise accept is delayed until the first allocation of the memory.

Yes, put that in the comment pls.

> Memory encryption can be a reason to have unaccepted memory, but it is not
> 1:1 match. Unaccepted memory can be present without memory ecnryption if
> data secruty and integrity guaranteed by other means.

Really?

Please elaborate. I thought memory acceptance is a feature solely for
TDX and SNP guests to use.

> <asm/mem_encrypt.h> is very AMD SME/SEV centric.

So?

> I'm not sure it need to exist in the way it is now.

I'm not sure what your argument actually is for having yet another
separate header vs putting it in a header which already deals with that
stuff.

> Okay, I will move it into a separate function, but it has to be called
> from allocate_e820() because it allocates and free the map.

You mean, you want for allocate_e820() to call this new function because
both allocate and free?

Might have to explain what you mean here exactly.

> > And you're saying that that efi_allocate_pages() below can really give a
> > 256M contiguous chunk?
> 
> Yes, that's assumption. Is it too high ask to deal with 4PiB of PA?

>From my experience, asking firmware to do stuff for ya is always a risky
thing. I guess such a huge allocation, when it fails, will be caught
early in platform verification so whatever...

-- 
Regards/Gruss,
    Boris.

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