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, 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.

Powered by blists - more mailing lists