linux-kernel - Re: [PATCHv8 06/14] 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] [day] [month] [year] [list]

Message-ID: <20230325005157.37b4alnuf6p6mh66@box.shutemov.name>
Date:   Sat, 25 Mar 2023 03:51:57 +0300
From:   "Kirill A. Shutemov" <kirill@...temov.name>
To:     Borislav Petkov <bp@...en8.de>
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>,
        Dario Faggioli <dfaggioli@...e.com>,
        Dave Hansen <dave.hansen@...el.com>,
        Mike Rapoport <rppt@...nel.org>,
        David Hildenbrand <david@...hat.com>,
        Mel Gorman <mgorman@...hsingularity.net>,
        marcelo.cerri@...onical.com, tim.gardner@...onical.com,
        khalid.elmously@...onical.com, philip.cox@...onical.com,
        aarcange@...hat.com, peterx@...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: [PATCHv8 06/14] efi/x86: Implement support for unaccepted memory

On Tue, Jan 03, 2023 at 03:20:55PM +0100, Borislav Petkov wrote:
> > diff --git a/drivers/firmware/efi/Kconfig b/drivers/firmware/efi/Kconfig
> > index 6787ed8dfacf..8aa8adf0bcb5 100644
> > --- a/drivers/firmware/efi/Kconfig
> > +++ b/drivers/firmware/efi/Kconfig
> > @@ -314,6 +314,20 @@ config EFI_COCO_SECRET
> >  	  virt/coco/efi_secret module to access the secrets, which in turn
> >  	  allows userspace programs to access the injected secrets.
> >  
> > +config UNACCEPTED_MEMORY
> > +	bool
> > +	depends on EFI_STUB
> 
> This still doesn't make a whole lotta sense. If I do "make menuconfig" I don't
> see the help text because that bool doesn't have a string prompt. So who is that
> help text for?

It is a form of documentation for a developer. The same happens for other
options. For instance, BOOT_VESA_SUPPORT or ARCH_HAS_CURRENT_STACK_POINTER.

Yes, it is not visible user, but I still think it is helpful for a
developer to understand what the option does.

> Then, in the last patch you have
> 
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -888,6 +888,8 @@ config INTEL_TDX_GUEST
>         select ARCH_HAS_CC_PLATFORM
>         select X86_MEM_ENCRYPT
>         select X86_MCE
> +       select UNACCEPTED_MEMORY
> +       select EFI_STUB
> 
> I guess you want to select UNACCEPTED_MEMORY only.

I had to rework it as

config INTEL_TDX_GUEST
	...
	depends on EFI_STUB
	select UNACCEPTED_MEMORY

Naked select UNACCEPTED_MEMORY doesn't work if EFI and EFI_STUB is
disabled:

WARNING: unmet direct dependencies detected for UNACCEPTED_MEMORY
  Depends on [n]: EFI [=n] && EFI_STUB [=n]
  Selected by [y]:
  - INTEL_TDX_GUEST [=y] && HYPERVISOR_GUEST [=y] && X86_64 [=y] && CPU_SUP_INTEL [=y] && X86_X2APIC [=y]

IIUC, the alternative is to have selects all the way down the option tree.

> 
> And I've already mentioned this whole mess:
> 
> https://lore.kernel.org/r/Yt%2BnOeLMqRxjObbx@zn.tnic
> 
> Please incorporate all review comments before sending a new version of
> your patch.
> 
> Ignoring review feedback is a very unfriendly thing to do:
> 
> - if you agree with the feedback, you work it in in the next revision
> 
> - if you don't agree, you *say* *why* you don't

Sorry, it was not my intention. I misread your comment and focused on
build issues around the option.

-- 
  Kiryl Shutsemau / Kirill A. Shutemov