linux-kernel - Re: [PATCH 0/8] memset() in crypto code

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: <CAMj1kXE5rZdUoNd4vqXDCuNbHuwapnxskU1pabQax4LVvUWxgg@mail.gmail.com>
Date:   Tue, 16 Nov 2021 15:41:00 +0100
From:   Ard Biesheuvel <ardb@...nel.org>
To:     Sandy Harris <sandyinchina@...il.com>
Cc:     LKML <linux-kernel@...r.kernel.org>,
        Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S. Miller" <davem@...emloft.net>
Subject: Re: [PATCH 0/8] memset() in crypto code

On Tue, 16 Nov 2021 at 12:20, Sandy Harris <sandyinchina@...il.com> wrote:
>
> Fairly often we want to clear some memory in crypto code; it holds
> things we are done using and do not want to leave lying around where
> an enemy might discover them. Typical examples are crypto keys or
> random numbers we have generated and used for output.
>
> The obvious way to do this is with memset(address,0,bytes) but there
> is a problem with that; because we are done using that memory, the
> compiler may optimise away the "useless" memset() call. Using
> memzero_explicit(address,bytes) instead solves the problem; that
> function is designed to resist the optimisation.
>
> There are well over 100 memset() calls in .c files in the crypto and
> security directories. I looked at them all and found about a dozen in
> eight files that I thought should be changed to memzero_explicit().
> Here they are as patches 1 to 8 in this series.
>
> I did read some code & think moderately carefully, but I do not know
> the code deeply & it is possible I have made some errors. I think
> false positives (making unnecessary changes) are more likely than
> false negatives (not catching necessary changes).

Hello Sandy,

As Greg alluded in reply to one of these patches, memzero_explicit()
is only usually needed for stack variables, because in those cases,
the compiler is able to infer that the memset() is the last thing that
touches the variable before it goes out of scope, and so memset()ing
it can be omitted.

Variables that are passed into a function by pointer reference have a
life time that is not known to the callee, and so there is no way the
compiler can elide memset() calls, which means that using
memzero_explicit() in such cases is not needed. The exception is
functions with static linkage that may end up being inlined into their
callers, but in the crypto subsystem, many such functions are invoked
indirectly via exported function pointers, which makes inlining
impossible.