linux-kernel - Re: [PATCH v2 4/4] virt: Add sev_secret module to expose confidential computing secrets

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: <290c21a8-a68f-0826-2754-1480f79a081d@intel.com>
Date:   Thu, 7 Oct 2021 06:48:54 -0700
From:   Dave Hansen <dave.hansen@...el.com>
To:     Dov Murik <dovmurik@...ux.ibm.com>, linux-efi@...r.kernel.org
Cc:     Borislav Petkov <bp@...e.de>, Ashish Kalra <ashish.kalra@....com>,
        Brijesh Singh <brijesh.singh@....com>,
        Tom Lendacky <thomas.lendacky@....com>,
        Ard Biesheuvel <ardb@...nel.org>,
        James Morris <jmorris@...ei.org>,
        "Serge E. Hallyn" <serge@...lyn.com>,
        Andi Kleen <ak@...ux.intel.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Andrew Scull <ascull@...gle.com>,
        "Dr. David Alan Gilbert" <dgilbert@...hat.com>,
        James Bottomley <jejb@...ux.ibm.com>,
        Tobin Feldman-Fitzthum <tobin@...ux.ibm.com>,
        Jim Cadden <jcadden@....com>,
        Daniele Buono <dbuono@...ux.vnet.ibm.com>,
        linux-coco@...ts.linux.dev, linux-security-module@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 4/4] virt: Add sev_secret module to expose confidential
 computing secrets

On 10/6/21 11:18 PM, Dov Murik wrote:
> +static void wipe_memory(void *addr, size_t size)
> +{
> +	memzero_explicit(addr, size);
> +	clean_cache_range(addr, size);
> +}

What's the purpose of the clean_cache_range()?  It's backed in a CLWB
instruction on x86 which seems like an odd choice.  I guess the point is
that the memzero_explicit() will overwrite the contents, but might have
dirty lines in the cache.  The CLWB will ensure that the lines are
actually written back to memory, clearing the secret out of memory.
Without the CLWB, the secret might live in memory until the dirtied
cachelines are written back.

Could you document this, please?  It would also be nice to include some
of this motivation in the patch that exports clean_cache_range() in the
first place.

I also think clean_cache_range() an odd choice.  If it were me, I
probably would have just used the already-exported
clflush_cache_range().  The practical difference between writing back
and flushing the cachelines is basically zero.  The lines will never be
reused.

*If* we export anything from x86 code, I think it should be something
which is specific to the task at hand, like arch_invalidate_pmem() is.

Also, when you are modifying x86 code, including exports, it would be
nice to include (all of) the x86 maintainers.  The relevant ones for
this series would probably be:

X86 ARCHITECTURE (32-BIT AND 64-BIT)
M:      Thomas Gleixner <tglx@...utronix.de>
M:      Ingo Molnar <mingo@...hat.com>
M:      Borislav Petkov <bp@...en8.de>
M:      x86@...nel.org

X86 MM
M:      Dave Hansen <dave.hansen@...ux.intel.com>
M:      Andy Lutomirski <luto@...nel.org>
M:      Peter Zijlstra <peterz@...radead.org>

There's also the handy dandy scripts/get_maintainer.pl to help.