Re: [PATCH 5.4 regression fix] x86/boot: Provide memzero_explicit

[Hans de Goede]

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.

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.

Regards,

Hans