Am Montag, 7. Oktober 2019, 11:06:04 CEST schrieb Hans de Goede:
Hi Hans,
> Hi Stephan,
>
> On 07-10-2019 10:59, Stephan Mueller wrote:
> > Am Montag, 7. Oktober 2019, 10:55:01 CEST schrieb Hans de Goede:
> >
> > Hi Hans,
> >
> >> The purgatory code now uses the shared lib/crypto/sha256.c sha256
> >> implementation. This needs memzero_explicit, implement this.
> >>
> >> Reported-by: Arvind Sankar <nivedita@xxxxxxxxxxxx>
> >> Fixes: 906a4bb97f5d ("crypto: sha256 - Use get/put_unaligned_be32 to get
> >> input, memzero_explicit") Signed-off-by: Hans de Goede
> >> <hdegoede@xxxxxxxxxx>
> >> ---
> >>
> >> arch/x86/boot/compressed/string.c | 5 +++++
> >> 1 file changed, 5 insertions(+)
> >>
> >> diff --git a/arch/x86/boot/compressed/string.c
> >> b/arch/x86/boot/compressed/string.c index 81fc1eaa3229..511332e279fe
> >> 100644
> >> --- a/arch/x86/boot/compressed/string.c
> >> +++ b/arch/x86/boot/compressed/string.c
> >> @@ -50,6 +50,11 @@ void *memset(void *s, int c, size_t n)
> >>
> >> return s;
> >>
> >> }
> >>
> >> +void memzero_explicit(void *s, size_t count)
> >> +{
> >> + memset(s, 0, count);
> >
> > May I ask how it is guaranteed that this memset is not optimized out by
> > the
> > compiler, e.g. for stack variables?
>
> The function and the caller live in different compile units, so unless
> LTO is used this cannot happen.
Agreed in this case.
I would just be worried that this memzero_explicit implementation is assumed
to be protected against optimization when used elsewhere since other
implementations of memzero_explicit are provided with the goal to be protected
against optimizations.
>
> Also note that the previous purgatory private (vs shared) sha256
> implementation had:
>
> /* Zeroize sensitive information. */
> memset(sctx, 0, sizeof(*sctx));
>
> In the place where the new shared 256 code uses memzero_explicit() and the
> new shared sha256 code is the only user of the
> arch/x86/boot/compressed/string.c memzero_explicit() implementation.
>
> With that all said I'm open to suggestions for improving this.
What speaks against the common memzero_explicit implementation? If you cannot
use it, what about adding a barrier in the memzero_explicit implementation? Or
what about adding some compiler magic as attached to this email?
>
> Regards,
>
> Hans
Ciao
Stephan/*
* Copyright (C) 2015, Stephan Mueller <smueller@xxxxxxxxxx>
*
* Redistribution and use in source and binary forms, with or without
* modification, are permitted provided that the following conditions
* are met:
* 1. Redistributions of source code must retain the above copyright
* notice, and the entire permission notice in its entirety,
* including the disclaimer of warranties.
* 2. Redistributions in binary form must reproduce the above copyright
* notice, this list of conditions and the following disclaimer in the
* documentation and/or other materials provided with the distribution.
* 3. The name of the author may not be used to endorse or promote
* products derived from this software without specific prior
* written permission.
*
* ALTERNATIVELY, this product may be distributed under the terms of
* the GNU General Public License, in which case the provisions of the GPL2
* are required INSTEAD OF the above restrictions. (This clause is
* necessary due to a potential bad interaction between the GPL and
* the restrictions contained in a BSD-style copyright.)
*
* THIS SOFTWARE IS PROVIDED ``AS IS'' AND ANY EXPRESS OR IMPLIED
* WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES
* OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE, ALL OF
* WHICH ARE HEREBY DISCLAIMED. IN NO EVENT SHALL THE AUTHOR BE
* LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
* CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT
* OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
* BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF
* LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE
* USE OF THIS SOFTWARE, EVEN IF NOT ADVISED OF THE POSSIBILITY OF SUCH
* DAMAGE.
*/
#include <string.h>
/*
* Tested following code:
*
* (1) __asm__ __volatile__("" : "=r" (s) : "0" (s));
* (2) __asm__ __volatile__("": : :"memory");
* (3) __asm__ __volatile__("" : "=r" (s) : "0" (s) : "memory");
* (4) __asm__ __volatile__("" : : "r" (s) : "memory");
*
* Requred result:
*
* gcc -O3: objdump -d shows the following:
*
* 0000000000400440 <main>:
* ...
* 400469: 48 c7 04 24 00 00 00 movq $0x0,(%rsp)
* 400470: 00
* 400471: 48 c7 44 24 08 00 00 movq $0x0,0x8(%rsp)
* 400478: 00 00
* 40047a: c7 44 24 10 00 00 00 movl $0x0,0x10(%rsp)
* 400481: 00
*
* clang -O3: objdump -d shows the following:
*
* 0000000000400590 <main>:
* ...
* 4005c3: c7 44 24 10 00 00 00 movl $0x0,0x10(%rsp)
* 4005ca: 00
*
*
* Test results:
*
* The following table marks an X when the aforementioned movq/movl code is
* present (or an invocation of memset@plt) in the object code
* (i.e. the code we want). Contrary, the table marks - where the code is not
* present (i.e. the code we do not want):
*
* | BARRIER | (1) | (2) | (3) | (4)
* ---------+----------+ | | |
* Compiler | | | | |
* =========+==========+=======================
* | | | |
* gcc -O0 | X | X | X | X
* | | | |
* gcc -O2 | - | X | X | X
* | | | |
* gcc -O3 | - | X | X | X
* | | | |
* clang -00 | X | X | X | X
* | | | |
* clang -02 | X | - | X | X
* | | | |
* clang -03 | - | - | X | X
*/
static inline void memset_secure(void *s, int c, size_t n)
{
memset(s, c, n);
__asm__ __volatile__("" : : "r" (s) : "memory");
}
#if 0
#include <stdio.h>
int main(int argc, char *argv[])
{
char buf[20];
snprintf(buf, sizeof(buf) - 1, "test");
printf("%s\n", buf);
memset_secure(buf, 0, sizeof(buf));
return 0;
}
#endif
