From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-qt1-f193.google.com ([209.85.160.193]:42423 "EHLO mail-qt1-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728079AbfJGSml (ORCPT ); Mon, 7 Oct 2019 14:42:41 -0400 From: Arvind Sankar Date: Mon, 7 Oct 2019 14:42:37 -0400 Subject: Re: [PATCH v2 5.4 regression fix] x86/boot: Provide memzero_explicit Message-ID: <20191007184237.GB13589@rani.riverdale.lan> References: <20191007134724.4019-1-hdegoede@redhat.com> <20191007140022.GA29008@gmail.com> <1dc3c53d-785e-f9a4-1b4c-3374c94ae0a7@redhat.com> <20191007142230.GA117630@gmail.com> <2982b666-e310-afb7-40eb-e536ce95e23d@redhat.com> <20191007144600.GB59713@gmail.com> <20191007152049.GA384920@rani.riverdale.lan> <20191007154007.GA96929@gmail.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="H+4ONPRPur6+Ovig" Content-Disposition: inline In-Reply-To: <20191007154007.GA96929@gmail.com> Sender: linux-s390-owner@vger.kernel.org List-ID: To: Ingo Molnar Cc: Arvind Sankar , Hans de Goede , Thomas Gleixner , Ingo Molnar , Borislav Petkov , "H . Peter Anvin" , Herbert Xu , Ard Biesheuvel , linux-crypto@vger.kernel.org, x86@kernel.org, linux-kernel@vger.kernel.org, Stephan Mueller , linux-s390@vger.kernel.org --H+4ONPRPur6+Ovig Content-Type: text/plain; charset=utf-8 Content-Disposition: inline On Mon, Oct 07, 2019 at 05:40:07PM +0200, Ingo Molnar wrote: > > * Arvind Sankar wrote: > > > With the barrier in there, is there any reason to *not* inline the > > function? barrier_data() is an asm statement that tells the compiler > > that the asm uses the memory that was set to zero, thus preventing it > > from removing the memset even if nothing else uses that memory later. A > > more detailed comment is there in compiler-gcc.h. I can't see why it > > wouldn't work even if it were inlined. > > > > If the function can indeed be inlined, we could just make the common > > implementation a macro and avoid duplicating it? As mentioned in another > > mail, we otherwise will likely need another duplicate implementation for > > arch/s390/purgatory as well. > > I suspect macro would be justified in this case. Mind sending a v3 patch > to demonstrate how it would all look like? > > I'll zap v2 if the macro solution looks better. > > Thanks, > > Ingo Patch attached to turn memzero_explicit into inline function. --H+4ONPRPur6+Ovig Content-Type: text/x-diff; charset=utf-8 Content-Disposition: attachment; filename="0001-lib-string-make-memzero_explicit-inline-instead-of-e.patch" >From 25834b8040eff72478489be0bd8a2ff549af7f94 Mon Sep 17 00:00:00 2001 From: Arvind Sankar Date: Mon, 7 Oct 2019 14:34:24 -0400 Subject: [PATCH] lib/string: make memzero_explicit inline instead of external With the use of the barrier implied by barrier_data(), there is no need for memzero_explicit to be extern. Making it inline saves the overhead of a function call, and allows the code to be reused in arch/*/purgatory without having to duplicate the implementation. Fixes: 906a4bb97f5d ("crypto: sha256 - Use get/put_unaligned_be32 to get input, memzero_explicit") Signed-off-by: Arvind Sankar --- include/linux/string.h | 21 ++++++++++++++++++++- lib/string.c | 21 --------------------- 2 files changed, 20 insertions(+), 22 deletions(-) diff --git a/include/linux/string.h b/include/linux/string.h index b2f9df7f0761..b6ccdc2c7f02 100644 --- a/include/linux/string.h +++ b/include/linux/string.h @@ -227,7 +227,26 @@ static inline bool strstarts(const char *str, const char *prefix) } size_t memweight(const void *ptr, size_t bytes); -void memzero_explicit(void *s, size_t count); + +/** + * memzero_explicit - Fill a region of memory (e.g. sensitive + * keying data) with 0s. + * @s: Pointer to the start of the area. + * @count: The size of the area. + * + * Note: usually using memset() is just fine (!), but in cases + * where clearing out _local_ data at the end of a scope is + * necessary, memzero_explicit() should be used instead in + * order to prevent the compiler from optimising away zeroing. + * + * memzero_explicit() doesn't need an arch-specific version as + * it just invokes the one of memset() implicitly. + */ +static inline void memzero_explicit(void *s, size_t count) +{ + memset(s, 0, count); + barrier_data(s); +} /** * kbasename - return the last part of a pathname. diff --git a/lib/string.c b/lib/string.c index cd7a10c19210..08ec58cc673b 100644 --- a/lib/string.c +++ b/lib/string.c @@ -748,27 +748,6 @@ void *memset(void *s, int c, size_t count) EXPORT_SYMBOL(memset); #endif -/** - * memzero_explicit - Fill a region of memory (e.g. sensitive - * keying data) with 0s. - * @s: Pointer to the start of the area. - * @count: The size of the area. - * - * Note: usually using memset() is just fine (!), but in cases - * where clearing out _local_ data at the end of a scope is - * necessary, memzero_explicit() should be used instead in - * order to prevent the compiler from optimising away zeroing. - * - * memzero_explicit() doesn't need an arch-specific version as - * it just invokes the one of memset() implicitly. - */ -void memzero_explicit(void *s, size_t count) -{ - memset(s, 0, count); - barrier_data(s); -} -EXPORT_SYMBOL(memzero_explicit); - #ifndef __HAVE_ARCH_MEMSET16 /** * memset16() - Fill a memory area with a uint16_t -- 2.21.0 --H+4ONPRPur6+Ovig--